Necessary change to fit mainline kernel.

25 views
Skip to first unread message

Ke Li

unread,
May 18, 2024, 10:39:44 AMMay 18
to Jailhouse
Hi, 
Per upstream change[1], it's necessary to adopt the new signature of of_overlay_fdt_apply if you are integrating with mainline kernel. Please consider approve [2].
If you feel necessary, it's also possible to defend this under some kind of version macro. I'm happy to provide a new revision of code.


By the way, if anyone else is also on the same page integrating with mainline kernel, I've done some patch [3] that is exactly done as the official jailhouse-enabling done here[4] (with minor tweaking so it consume latest in-kernel API).
KL

Ralf Ramsauer

unread,
May 23, 2024, 6:58:43 AMMay 23
to Ke Li, Jailhouse
Hi,

On 23/05/2024 09:43, Ke Li wrote:
> Hi, Ralf,
> I've taken your advice and do the following:
> 1. Revert the change on JAILHOUSE_BASE to 0xffffc0200000. (btw, I got
> the idea of modifying it from the talk "Tutorial: Bootstrapping the
> Partitioning Hypervisor Jailhouse"
> <https://www.youtube.com/watch?v=7fiJbwmhnRw
> <https://www.youtube.com/watch?v=7fiJbwmhnRw>> time stamp 1:28:31)

Outdated. Don't touch the code at the moment, just configuration.

> 2. Update the RootCellConfig and add whatever memory region I can find
> (except gic-v2). Info collected from below, the latest version can be
> found here:
> https://github.com/siemens/jailhouse/compare/master...cnnblike:jailhouse-rk:working-branch <https://github.com/siemens/jailhouse/compare/master...cnnblike:jailhouse-rk:working-branch>
>   a. from RK3308 Technical Programming Manual p12-p13
> (https://dl.radxa.com/rockpis/docs/hw/datasheets/Rockchip%20RK3308TRM%20V1.1%20Part1-20180810.pdf <https://dl.radxa.com/rockpis/docs/hw/datasheets/Rockchip%20RK3308TRM%20V1.1%20Part1-20180810.pdf>)
>   b. the live dts I pulled from a working armbian kernel (you can find
> it from
> https://gist.github.com/cnnblike/f758596d59899f4d300eefb55ef5f81e
> <https://gist.github.com/cnnblike/f758596d59899f4d300eefb55ef5f81e>)

Please no links, just simple attachments in the future. It's horrible to
clock-navigate through github to get what i need: raw content.

>   c. the iomem result:
> https://gist.github.com/cnnblike/eb6ba4ce958d058edb0b7ae4ddd124e5
> <https://gist.github.com/cnnblike/eb6ba4ce958d058edb0b7ae4ddd124e5>

Alright, just a short analysis: Jailhouse reservation is placed
correctly. iomem confirms the reservation. Hypervisor base addresses in
the configuration are correct. So there's nothing odd here.

> 3. I have also done multiple experiments on my side, all leading to "no
> output" result except the following that may help in troubleshooting:
>   a. I changed the cpus section from 0b1111 to 0b0111, thinking maybe
> one leftover core could at least allow the kernel to have a chance to
> emit something for diagnosis. It turns out it DID emit something with
> one core left:

Nonono, don't do that! Jailhouse needs all CPUs. That won't work!

> https://gist.github.com/cnnblike/34dd8d241846c8b8cf43f01cc4124dcf
> <https://gist.github.com/cnnblike/34dd8d241846c8b8cf43f01cc4124dcf>
>   b. map the peripheral memory region as a whole
> (0xff000000~0xfffdffff, sram configed same as ram) to physic address, no
> output.
>   c. same as b but without gic, no output.

GIC seems to be configured correctly.

> With so many experiment, I'm wondering, if it could be some other reason
> behind such weird freeze kernel result.

So the thing is, no matter what mistakes in the memory region you might
have in your configuration, you should at least see a Hello world from
the hypervisor on the UART. That's the first thing we need to get
running. The rest is something for later.

So why doesn't the UART work?

You already confirmed, that echoing to /dev/ttysomething on that line
works, right? Are you *absolutely* sure that you use the right uart
line? Please attach output of dmesg.

Anyway, base address and size of the UART are fine... In another
project, some time ago, I also had some issue with the designware UARTs
(8250_dw). There, it was the Fifo Control Register for some reason.
Let's try that. And let's print a early 'X' to see if we get here.

Could you please try the patch below?


Ralf



diff --git a/hypervisor/uart-8250.c b/hypervisor/uart-8250.c
index e6112820..7b29af0e 100644
--- a/hypervisor/uart-8250.c
+++ b/hypervisor/uart-8250.c
@@ -19,6 +19,7 @@
#define UART_TX 0x0
#define UART_DLL 0x0
#define UART_DLM 0x1
+#define UART_FCR 0x2
#define UART_LCR 0x3
#define UART_LCR_8N1 0x03
#define UART_LCR_DLAB 0x80
@@ -54,6 +55,10 @@ static void uart_init(struct uart_chip *chip)
chip->reg_in = reg_in_mmio8;
}

+ chip->reg_out(chip, UART_FCR, 0);
+ chip->reg_out(chip, UART_LCR, UART_LCR_8N1);
+ chip->reg_out(chip, UART_TX, 'X');
+
/* only initialise if divider is not zero */
if (!chip->debug_console->divider)
return;

Ke Li

unread,
May 23, 2024, 11:26:07 AMMay 23
to Ralf Ramsauer, Jailhouse
Hi, Ralf,

I've double verified that the UART is working as expected by "root@rockpi-s:~# echo "random test text" > /dev/ttyS1" and I DID receive corresponding text on work machine, this not only validated the dts setup is correct but also the USB-UART converter and the connection between them are all as expected.

Dmesg is also attached just in case you need to check if anything is suspicious.

I checked the mail-list on kernel.org around potential issues with the DesignWave based 8250-UART IP from RockChip, found this[1]. Another chat behind the difference should be this one [2] around the AllWinner difference from RockChip, I may take a deep investigation into the detail behind this and let you know.

Tried applying the patch you mentioned, still no text came out - which also surprises me. 

dmesg.txt
2024-05-23_23-05.png
patched code still fail.txt

Ralf Ramsauer

unread,
May 23, 2024, 12:26:01 PMMay 23
to Ke Li, Jailhouse
Hi,

On 23/05/2024 17:25, Ke Li wrote:
> Hi, Ralf,
>
> I've double verified that the UART is working as expected by
> "root@rockpi-s:~# echo "random test text" > /dev/ttyS1" and I DID
> receive corresponding text on work machine, this not only validated the
> dts setup is correct but also the USB-UART converter and the connection
> between them are all as expected.

Alright, ttyS1 in deed is 0xff0b0000:
[ 0.946678] ff0b0000.serial: ttyS1 at MMIO 0xff0b0000 (irq = 28,
base_baud = 5078125) is a 16550A

Ok, so we don't have an issue here. I'm just looking for common mistakes.

>
> Dmesg is also attached just in case you need to check if anything is
> suspicious.
>
> I checked the mail-list on kernel.org <http://kernel.org> around
> potential issues with the DesignWave based 8250-UART IP from RockChip,
> found this[1]. Another chat behind the difference should be this one [2]
> around the AllWinner difference from RockChip, I may take a deep
> investigation into the detail behind this and let you know.
>
> Tried applying the patch you mentioned, still no text came out - which
> also surprises me.

Ok, crap. Now it's getting difficult. The thing is, we don't know yet
what is wrong: We can either have an issue with the UART driver, or,
what is also possible, we might already crash /before/ we initialise the
UART. Without any possibility of interaction with the device it's hard
to find out what happens.

Next thing I would try is to try to write to the UART from inside kernel
space, without starting the hypervisor. Just to know where we hang.
Would you please try (untested pseudocode, but you should get the idea):


diff --git a/driver/main.c b/driver/main.c
index c8572470..e4482181 100644
--- a/driver/main.c
+++ b/driver/main.c
@@ -585,6 +585,14 @@ static int jailhouse_cmd_enable(struct
jailhouse_system __user *arg)

error_code = 0;

+ pr("Mapping console %lx\n", config->debug_console.address);
+ console = ioremap(config->debug_console.address,
config->debug_console.size);
+ if (!console) {
+ pr("Error!\n");
+ } else {
+ writel('X', console + 0x0);
+ }
+
+
preempt_disable();


Can you see the X on the serial line?

Ralf


>
> [1]https://patchwork.kernel.org/project/linux-rockchip/patch/20170206233000....@chromium.org/ <https://patchwork.kernel.org/project/linux-rockchip/patch/20170206233000....@chromium.org/>
> <mailto:ralf.r...@oth-regensburg.de>> wrote:
>
> Hi,
>
> On 23/05/2024 09:43, Ke Li wrote:
> > Hi, Ralf,
> > I've taken your advice and do the following:
> > 1. Revert the change on JAILHOUSE_BASE to 0xffffc0200000. (btw, I
> got
> > the idea of modifying it from the talk "Tutorial: Bootstrapping the
> > Partitioning Hypervisor Jailhouse"
> > <https://www.youtube.com/watch?v=7fiJbwmhnRw
> <https://www.youtube.com/watch?v=7fiJbwmhnRw>
> > <https://www.youtube.com/watch?v=7fiJbwmhnRw
> <https://www.youtube.com/watch?v=7fiJbwmhnRw>>> time stamp 1:28:31)
>
> Outdated. Don't touch the code at the moment, just configuration.
>
> > 2. Update the RootCellConfig and add whatever memory region I can
> find
> > (except gic-v2). Info collected from below, the latest version
> can be
> > found here:
> >
> https://github.com/siemens/jailhouse/compare/master...cnnblike:jailhouse-rk:working-branch <https://github.com/siemens/jailhouse/compare/master...cnnblike:jailhouse-rk:working-branch> <https://github.com/siemens/jailhouse/compare/master...cnnblike:jailhouse-rk:working-branch <https://github.com/siemens/jailhouse/compare/master...cnnblike:jailhouse-rk:working-branch>>
> >    a. from RK3308 Technical Programming Manual p12-p13
> >
> (https://dl.radxa.com/rockpis/docs/hw/datasheets/Rockchip%20RK3308TRM%20V1.1%20Part1-20180810.pdf <https://dl.radxa.com/rockpis/docs/hw/datasheets/Rockchip%20RK3308TRM%20V1.1%20Part1-20180810.pdf> <https://dl.radxa.com/rockpis/docs/hw/datasheets/Rockchip%20RK3308TRM%20V1.1%20Part1-20180810.pdf <https://dl.radxa.com/rockpis/docs/hw/datasheets/Rockchip%20RK3308TRM%20V1.1%20Part1-20180810.pdf>>)
Reply all
Reply to author
Forward
Message has been deleted
0 new messages