On 27.05.20 16:09, Nikhil Devshatwar wrote:
> 
> 
> On 27/05/20 6:55 pm, Jan Kiszka wrote:
>> On 27.05.20 14:32, nikhil...@ti.com wrote:
>>> From: Nikhil Devshatwar <nikhil...@ti.com>
>>>
>>> Add a virtual PCI device with IVSHMEM type (id = 1)
>>> Create IVSHMEM regions for 2 peer communication
>>> Enable the vpci_irq for doorbell interrupt
>>>
>>> This allows to run the ivshmem-demo baremetal inmate
>>> inside this cell.
>>>
>>> Signed-off-by: Nikhil Devshatwar <nikhil...@ti.com>
>>> ---
>>>   configs/arm64/k3-j721e-evm-inmate-demo.c | 63 ++++++++++++++++++++++--
>>>   1 file changed, 59 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/configs/arm64/k3-j721e-evm-inmate-demo.c
>>> b/configs/arm64/k3-j721e-evm-inmate-demo.c
>>> index 7440a258..e09c4850 100644
>>> --- a/configs/arm64/k3-j721e-evm-inmate-demo.c
>>> +++ b/configs/arm64/k3-j721e-evm-inmate-demo.c
>>> @@ -20,7 +20,9 @@
>>>   struct {
>>>       struct jailhouse_cell_desc cell;
>>>       __u64 cpus[1];
>>> -    struct jailhouse_memory mem_regions[3];
>>> +    struct jailhouse_memory mem_regions[7];
>>> +    struct jailhouse_irqchip irqchips[1];
>>> +    struct jailhouse_pci_device pci_devices[1];
>>>   } __attribute__((packed)) config = {
>>>       .cell = {
>>>           .signature = JAILHOUSE_CELL_DESC_SIGNATURE,
>>> @@ -30,8 +32,9 @@ struct {
>>>             .cpu_set_size = sizeof(config.cpus),
>>>           .num_memory_regions = ARRAY_SIZE(config.mem_regions),
>>> -        .num_irqchips = 0,
>>> -        .num_pci_devices = 0,
>>> +        .num_irqchips = 1,
>>> +        .num_pci_devices = 1,
>>> +        .vpci_irq_base = 195 -32,
>>>             .console = {
>>>               .address = 0x02810000,
>>> @@ -48,6 +51,33 @@ struct {
>>>       },
>>>         .mem_regions = {
>>> +        /* IVSHMEM shared memory regions for 00:00.0 (demo) */
>>> +        {
>>> +            .phys_start = 0x89fe00000,
>>> +            .virt_start = 0x89fe00000,
>>> +            .size = 0x10000,
>>
>> This means the kernel is configured for 64K pages - rather uncommon and
>> problematic in many regards (just had to turn it off on AM65x because it
>> breaks userspace [1]).
> 
> Yes, indeed.
> In fact, I was planning to send one more patch on ivshemem_uio to map
> a page aligned register memory[0]
> 
> 
> At TI, Jailhouse is integrated with TI linux kernel[1] (which is
> different from the siemens kernel[2])
> 
> Can you please point me details of failures with 64k kernel.
> This will help me to convince TI kernel developers to turn off 64k
> paging because of whatever issues that might occur.
> 
> 
> [0] =
> https://git.ti.com/cgit/ti-linux-kernel/ti-linux-kernel/commit/?h=ti-linux-5.4.y&id=30061ab0dcd216ccf20fa9bc426b575feaaefcb4
> 
> [1] =
> https://git.ti.com/cgit/ti-linux-kernel/ti-linux-kernel/log/?h=ti-linux-5.4.y
> 
> [2] = https://github.com/siemens/linux/tree/jailhouse-enabling/5.4
> 
> 
> Regards,
> Nikhil D
> 
> 
>>
>>> +            .flags = JAILHOUSE_MEM_READ | JAILHOUSE_MEM_ROOTSHARED,
>>> +        },
>>> +        {
>>> +            .phys_start = 0x89fe10000,
>>> +            .virt_start = 0x89fe10000,
>>> +            .size = 0x10000,
> [snip]
>>> +    },
>>> +
>>> +    .pci_devices = {
>>> +        /* 00:00.0 */ {
>>> +            .type = JAILHOUSE_PCI_TYPE_IVSHMEM,
>>> +            .bdf = 0 << 3,
>>> +            .bar_mask = JAILHOUSE_IVSHMEM_BAR_MASK_INTX,
>>
>> This constant assumes 4K pages.
> 
> Shall I create another macro like below?
> 
> #define JAILHOUSE_IVSHMEM_BAR_MASK_INTX                 \
>         {                                               \
>                 0xffff0000, 0x00000000, 0x00000000,     \
>                 0x00000000, 0x00000000, 0x00000000,     \
>         }
> 
> Also, as long as the memory that is getting exposed by the registers
> regions is less than 4k, it should work right?
> Maybe I won't need the above patch if I do this change.
> 
> 
>>
>> Given all the problems with 64K, I suspect it's better to switch to
>> standard 4K.
>>
> 
> I am afraid that might need lot of convincing.

If you have convincing performance numbers on 64 vs. 4K, I'm also
curious. :)

Findings so far:
 - https://github.com/siemens/meta-iot2050/pull/7
 - RT seems to have latency problems, that's why you already disable it
   in your config
 - in the past at least, btrfs had a page-size dependency, making it
   impossible to mount it on a 4K-machine when it was created for 64K

And I'm sure there is more. IIRC, SLES used to be 64K on ARM64 but gave
up as well.

> Till now, I am able to get the Jailhouse and ivshmem working nicely with
> 64k pages. Need to check if I have introduced any security holes, etc

As pointed out, the MMIO register region is configured by default for 4K
pages. The PCI BAR mask can be adjusted to account for larger pages,
though, but that is missing in your patches yet. Jailhouse should be
fine with that, it was "just" not tested so far.

Jan

-- 
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux

-- 
You received this message because you are subscribed to the Google Groups 
"Jailhouse" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to jailhouse-dev+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/jailhouse-dev/68e2bf7f-cd61-47ed-5ef1-f928d010a95c%40siemens.com.

Reply via email to