Re: [PATCH v3] hw/arm/virt: Avoid unexpected warning from Linux guest on host with Fujitsu CPUs

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 17/6/24 07:26, Zhenyu Zhang wrote:
> On Thu, Jun 13, 2024 at 1:48 AM Robin Murphy <robin.murphy@arm.com> wrote:
>>
>> On 2024-06-12 1:50 pm, Philippe Mathieu-Daudé wrote:
>>> On 12/6/24 14:48, Peter Maydell wrote:
>>>> On Wed, 12 Jun 2024 at 13:33, Philippe Mathieu-Daudé
>>>> <philmd@linaro.org> wrote:
>>>>>
>>>>> Hi Zhenyu,
>>>>>
> Hello Philippe,
> 
>>>>> On 12/6/24 04:05, Zhenyu Zhang wrote:
>>>>>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>>>>>> index 3c93c0c0a6..3cefac6d43 100644
>>>>>> --- a/hw/arm/virt.c
>>>>>> +++ b/hw/arm/virt.c
>>>>>> @@ -271,6 +271,17 @@ static void create_fdt(VirtMachineState *vms)
>>>>>>         qemu_fdt_setprop_cell(fdt, "/", "#size-cells", 0x2);
>>>>>>         qemu_fdt_setprop_string(fdt, "/", "model", "linux,dummy-virt");
>>>>>>
>>>>>> +    /*
>>>>>> +     * For QEMU, all DMA is coherent. Advertising this in the root
>>>>>> node
>>>>>> +     * has two benefits:
>>>>>> +     *
>>>>>> +     * - It avoids potential bugs where we forget to mark a DMA
>>>>>> +     *   capable device as being dma-coherent
>>>>>> +     * - It avoids spurious warnings from the Linux kernel about
>>>>>> +     *   devices which can't do DMA at all
>>>>>> +     */
>>>>>> +    qemu_fdt_setprop(fdt, "/", "dma-coherent", NULL, 0);
>>>>>
>>>>> OK, but why restrict that to the Aarch64 virt machine?
>>>>> Shouldn't advertise this generically in create_device_tree()?
>>>>> Or otherwise at least in the other virt machines?
>>>>
>>>> create_device_tree() creates an empty device tree, not one
>>>> with stuff in it. It seems reasonable to me for this property
>>>> on the root to be set in the same place we set other properties
>>>> of the root node.
>>>
>>> OK. Still the question about other virt machines remains
>>> unanswered :)
>>
>>   From the DT consumer point of view, the interpretation and assumptions
>> around coherency *are* generally architecture- or platform-specific. For
>> instance on RISC-V, many platforms want to assume coherency by default
>> (and potentially use "dma-noncoherent" to mark individual devices that
>> aren't), while others may still want to do the opposite and use
>> "dma-coherent" in the same manner as Arm and AArch64. Neither property
>> existed back in ePAPR, so typical PowerPC systems wouldn't even be
>> looking and will just make their own assumptions by other means.

TIL :)

> As Robin's comment says, each platform wants to assume coherency
> by default may be different. Adding it to all virt machines may
> introduce new risks. Currently, the issue is only valid on Fujitsu CPUs
> where the cache line size is 256 bytes, meaning the combination of
> kvm+virt-platform is needed to trigger the warning. So I'd be inclined
> to add this "dma-coherent" property for the "virt" platform first
> and advertise the property to other platforms if we hit the issue
> on those platforms.

Thanks Robin & Zhenyu,

Phil.