HYPERVISOR_console_io + CONSOLEIO_write needs COPY_flush_dcache

HYPERVISOR_console_io + CONSOLEIO_write needs COPY_flush_dcache

[Oleksandr Andrushchenko]

Hello,

While adding support for para-virtualized block device in u-boot

I faced an issue with HYPERVISOR_console_io + CONSOLEIO_write.

I tried to use the hypercall during MMU setup stage while enabling data cache

on arm64 platform (e.g. data cache is not yet enabled) and saw in guest's console

either old data or some sort of garbage. Printing constant strings, just like mini-os

does on boot, works as expected. So, looking at the Xen code [1] it seems

that we should copy guest's buffer with COPY_flush_dcache set in this case

as (usually?) this hypercall is used in guest's code which doesn't have any

other means to print yet, e.g. early printk case.

If my understanding of the issue correct then I can probably prepare a patch.

Thank you,

Oleksandr

[1] https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/drivers/char/console.c;h=56e24821b2f8d2b9bfb99d2acb721b06277834ce;hb=refs/heads/master#l597

Re: HYPERVISOR_console_io + CONSOLEIO_write needs COPY_flush_dcache

[Julien Grall]

On 10/06/2020 09:13, Oleksandr Andrushchenko wrote:
> Hello,

Hi,

> While adding support for para-virtualized block device in u-boot
> 
> I faced an issue with HYPERVISOR_console_io + CONSOLEIO_write.
> 
> I tried to use the hypercall during MMU setup stage while enabling data cache
> 
> on arm64 platform (e.g. data cache is not yet enabled) and saw in guest's console
> 
> either old data or some sort of garbage. Printing constant strings, just like mini-os
> 
> does on boot, works as expected. 

Per the hypercall ABI (see include/public/arch-arm.h) all the buffers 
must reside in memory which is mapped as Normal Inner Write-Back 
Inner-Shareable.

You are passing non-cacheable memory, so the behavior you see is expected.

> So, looking at the Xen code [1] it seems
> 
> that we should copy guest's buffer with COPY_flush_dcache set in this case

COPY_flush_dcache is only meant to be used when copy to guest memory to 
make sure the data reached the point of coherency in your system. It is 
not meant to be used when copying from guest.

> as (usually?) this hypercall is used in guest's code which doesn't have any
> other means to print yet, e.g. early printk case.

I have been using it after the MMU is setup but before the console is 
properly setup by the guest (there is a lot that can happen in Linux 
between the two). In my case, the flush is not necessary and would be wrong.

In general, the cache is never off on Arm64 because you may have system 
cache or, in the cache of virtualization, cacheable mapping in the 
hypervisor (all the memory is mapped).

When updating data with MMU/D-cache off the normal approach is:
    1) Remove any dirty lines from the cache, otherwise they may get 
evicted while updating the memory and override any value written with 
MMU off.
    2) Update the memory
    3) Remove any lines that may have been speculated so when you turn 
onthe MMU and D-cache, U-boot can obverse the correct data.

Step 1 is only necessary if you are going to write outside of the loaded 
Image (BSS is not part of it).

You most likely already have similar steps in U-boot for other part of 
the memory access with MMU/D-Cache off. So I think U-boot is the best 
place to invalidate the cache before issuing the hypercall.

Cheers,

-- 
Julien Grall,

Re: HYPERVISOR_console_io + CONSOLEIO_write needs COPY_flush_dcache

[Oleksandr Andrushchenko]

On 6/10/20 3:50 PM, Julien Grall wrote:
> On 10/06/2020 09:13, Oleksandr Andrushchenko wrote:
>> Hello,
>
> Hi,
>
>> While adding support for para-virtualized block device in u-boot
>>
>> I faced an issue with HYPERVISOR_console_io + CONSOLEIO_write.
>>
>> I tried to use the hypercall during MMU setup stage while enabling data cache
>>
>> on arm64 platform (e.g. data cache is not yet enabled) and saw in guest's console
>>
>> either old data or some sort of garbage. Printing constant strings, just like mini-os
>>
>> does on boot, works as expected. 
>
> Per the hypercall ABI (see include/public/arch-arm.h) all the buffers must reside in memory which is mapped as Normal Inner Write-Back Inner-Shareable.
>
> You are passing non-cacheable memory, so the behavior you see is expected.
This is what we come up with as well
>
>> So, looking at the Xen code [1] it seems
>>
>> that we should copy guest's buffer with COPY_flush_dcache set in this case
>
> COPY_flush_dcache is only meant to be used when copy to guest memory to make sure the data reached the point of coherency in your system. It is not meant to be used when copying from guest.
Understood and agree
>
>> as (usually?) this hypercall is used in guest's code which doesn't have any
>> other means to print yet, e.g. early printk case.
>
> I have been using it after the MMU is setup 
The thing is that in u-boot a lot of things happen before the MMU setup...
> but before the console is properly setup by the guest (there is a lot that can happen in Linux between the two). In my case, the flush is not necessary and would be wrong.
... and without the flush you lose the ability to debug that code.
>
> In general, the cache is never off on Arm64 because you may have system cache or, in the cache of virtualization, cacheable mapping in the hypervisor (all the memory is mapped).
>
> When updating data with MMU/D-cache off the normal approach is:
>    1) Remove any dirty lines from the cache, otherwise they may get evicted while updating the memory and override any value written with MMU off.
>    2) Update the memory
>    3) Remove any lines that may have been speculated so when you turn onthe MMU and D-cache, U-boot can obverse the correct data.
>
> Step 1 is only necessary if you are going to write outside of the loaded Image (BSS is not part of it).
>
> You most likely already have similar steps in U-boot for other part of the memory access with MMU/D-Cache off. So I think U-boot is the best place to invalidate the cache before issuing the hypercall.

Yes, I have invalidated the buffers and everything works like a charm now, thanks

Probably my use-case should be somewhere documented, so another time someone steps into similar issues it is explained.

>
> Cheers,
>
Thank you for helping,

Oleksandr

Re: HYPERVISOR_console_io + CONSOLEIO_write needs COPY_flush_dcache

[Julien Grall]

Hi,

On 11/06/2020 13:18, Oleksandr Andrushchenko wrote:
> 
> On 6/10/20 3:50 PM, Julien Grall wrote:
>> On 10/06/2020 09:13, Oleksandr Andrushchenko wrote:
>>> Hello,
>>
>> Hi,
>>
>>> While adding support for para-virtualized block device in u-boot
>>>
>>> I faced an issue with HYPERVISOR_console_io + CONSOLEIO_write.
>>>
>>> I tried to use the hypercall during MMU setup stage while enabling data cache
>>>
>>> on arm64 platform (e.g. data cache is not yet enabled) and saw in guest's console
>>>
>>> either old data or some sort of garbage. Printing constant strings, just like mini-os
>>>
>>> does on boot, works as expected.
>>
>> Per the hypercall ABI (see include/public/arch-arm.h) all the buffers must reside in memory which is mapped as Normal Inner Write-Back Inner-Shareable.
>>
>> You are passing non-cacheable memory, so the behavior you see is expected.
> This is what we come up with as well
>>
>>> So, looking at the Xen code [1] it seems
>>>
>>> that we should copy guest's buffer with COPY_flush_dcache set in this case
>>
>> COPY_flush_dcache is only meant to be used when copy to guest memory to make sure the data reached the point of coherency in your system. It is not meant to be used when copying from guest.
> Understood and agree
>>
>>> as (usually?) this hypercall is used in guest's code which doesn't have any
>>> other means to print yet, e.g. early printk case.
>>
>> I have been using it after the MMU is setup
> The thing is that in u-boot a lot of things happen before the MMU setup...
>> but before the console is properly setup by the guest (there is a lot that can happen in Linux between the two). In my case, the flush is not necessary and would be wrong.
> ... and without the flush you lose the ability to debug that code.

This is not entirely correct... You have other facility in Xen to debug 
early code (i.e pl011, hvc 0xffxx) and doesn't require any flush.

>>
>> In general, the cache is never off on Arm64 because you may have system cache or, in the cache of virtualization, cacheable mapping in the hypervisor (all the memory is mapped).
>>
>> When updating data with MMU/D-cache off the normal approach is:
>>     1) Remove any dirty lines from the cache, otherwise they may get evicted while updating the memory and override any value written with MMU off.
>>     2) Update the memory
>>     3) Remove any lines that may have been speculated so when you turn onthe MMU and D-cache, U-boot can obverse the correct data.
>>
>> Step 1 is only necessary if you are going to write outside of the loaded Image (BSS is not part of it).
>>
>> You most likely already have similar steps in U-boot for other part of the memory access with MMU/D-Cache off. So I think U-boot is the best place to invalidate the cache before issuing the hypercall.
> 
> Yes, I have invalidated the buffers and everything works like a charm now, thanks
> 
> Probably my use-case should be somewhere documented, so another time someone steps into similar issues it is explained.

You would have the exact same issue if you don't use hypercalls but 
modify a buffer with MMU off and then expect it to read the content 
correctly after turning on the MMU. That's even on baremetal! (see [1])

I am afraid you have to know how the cache works when writing code that 
will run with MMU off.

Therefore I don't think it is Xen Project to document how an 
architecture works. Instead this should be a tutorial for anyone wanting 
to write its own OS.

Cheers,

[1] https://www.youtube.com/watch?v=A_zCxzpxzmE

-- 
Julien Grall

Re: HYPERVISOR_console_io + CONSOLEIO_write needs COPY_flush_dcache

[Stefano Stabellini]

On Thu, 11 Jun 2020, Julien Grall wrote:
> > Probably my use-case should be somewhere documented, so another time someone
> > steps into similar issues it is explained.
> 
> You would have the exact same issue if you don't use hypercalls but modify a
> buffer with MMU off and then expect it to read the content correctly after
> turning on the MMU. That's even on baremetal! (see [1])
> 
> I am afraid you have to know how the cache works when writing code that will
> run with MMU off.
> 
> Therefore I don't think it is Xen Project to document how an architecture
> works. Instead this should be a tutorial for anyone wanting to write its own
> OS.

We could have a page on wiki.xenproject.org on the topic if Oleksandr
wants to write it. (Adding George in case Oleksandr wants to write the
and he doesn't have access to the wiki yet.)