[PATCH for-9.1 1/2] Revert "hw/riscv/virt.c: imsics DT: add '#msi-cells'"

Andrew Jones posted 2 patches 3 months, 1 week ago
[PATCH for-9.1 1/2] Revert "hw/riscv/virt.c: imsics DT: add '#msi-cells'"
Posted by Andrew Jones 3 months, 1 week ago
This reverts commit f42cdf2ea5b3a1dc369792d7acbf9cd3e5c90815.

Linux does not properly handle '#msi-cells=<0>' when searching for
MSI controllers for PCI devices which results in the devices being
unable to use MSIs. A patch for Linux has been sent[1] but until it,
or something like it, is merged and in distro kernels we should stop
adding the property. It's harmless to stop adding it since the
absence of the property and a value of zero for the property mean
the same thing according to the DT binding definition.

Link: https://lore.kernel.org/all/20240816124957.130017-2-ajones@ventanamicro.com/ # 1
Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
---
 hw/riscv/virt.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 9981e0f6c9b9..cef41c150aaf 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -552,7 +552,6 @@ static void create_fdt_one_imsic(RISCVVirtState *s, hwaddr base_addr,
                           FDT_IMSIC_INT_CELLS);
     qemu_fdt_setprop(ms->fdt, imsic_name, "interrupt-controller", NULL, 0);
     qemu_fdt_setprop(ms->fdt, imsic_name, "msi-controller", NULL, 0);
-    qemu_fdt_setprop_cell(ms->fdt, imsic_name, "#msi-cells", 0);
     qemu_fdt_setprop(ms->fdt, imsic_name, "interrupts-extended",
                      imsic_cells, ms->smp.cpus * sizeof(uint32_t) * 2);
     qemu_fdt_setprop(ms->fdt, imsic_name, "reg", imsic_regs,
-- 
2.45.2
Re: [PATCH for-9.1 1/2] Revert "hw/riscv/virt.c: imsics DT: add '#msi-cells'"
Posted by Alistair Francis 3 months, 1 week ago
On Sat, Aug 17, 2024 at 2:08 AM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> This reverts commit f42cdf2ea5b3a1dc369792d7acbf9cd3e5c90815.
>
> Linux does not properly handle '#msi-cells=<0>' when searching for
> MSI controllers for PCI devices which results in the devices being
> unable to use MSIs. A patch for Linux has been sent[1] but until it,
> or something like it, is merged and in distro kernels we should stop
> adding the property. It's harmless to stop adding it since the
> absence of the property and a value of zero for the property mean
> the same thing according to the DT binding definition.
>
> Link: https://lore.kernel.org/all/20240816124957.130017-2-ajones@ventanamicro.com/ # 1
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
>  hw/riscv/virt.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index 9981e0f6c9b9..cef41c150aaf 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -552,7 +552,6 @@ static void create_fdt_one_imsic(RISCVVirtState *s, hwaddr base_addr,
>                            FDT_IMSIC_INT_CELLS);
>      qemu_fdt_setprop(ms->fdt, imsic_name, "interrupt-controller", NULL, 0);
>      qemu_fdt_setprop(ms->fdt, imsic_name, "msi-controller", NULL, 0);
> -    qemu_fdt_setprop_cell(ms->fdt, imsic_name, "#msi-cells", 0);
>      qemu_fdt_setprop(ms->fdt, imsic_name, "interrupts-extended",
>                       imsic_cells, ms->smp.cpus * sizeof(uint32_t) * 2);
>      qemu_fdt_setprop(ms->fdt, imsic_name, "reg", imsic_regs,
> --
> 2.45.2
>
>
Re: [PATCH for-9.1 1/2] Revert "hw/riscv/virt.c: imsics DT: add '#msi-cells'"
Posted by Daniel Henrique Barboza 3 months, 1 week ago

On 8/16/24 1:07 PM, Andrew Jones wrote:
> This reverts commit f42cdf2ea5b3a1dc369792d7acbf9cd3e5c90815.
> 
> Linux does not properly handle '#msi-cells=<0>' when searching for
> MSI controllers for PCI devices which results in the devices being
> unable to use MSIs. A patch for Linux has been sent[1] but until it,
> or something like it, is merged and in distro kernels we should stop
> adding the property. It's harmless to stop adding it since the
> absence of the property and a value of zero for the property mean
> the same thing according to the DT binding definition.
> 
> Link: https://lore.kernel.org/all/20240816124957.130017-2-ajones@ventanamicro.com/ # 1
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> ---

Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>


This is indeed a 9.1 fix. Thanks Drew for sending it.

We can discuss whether we should wrap this around the 'strict-dt' flag or not, but
that can wait for 9.2.


Thanks,

Daniel


>   hw/riscv/virt.c | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index 9981e0f6c9b9..cef41c150aaf 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -552,7 +552,6 @@ static void create_fdt_one_imsic(RISCVVirtState *s, hwaddr base_addr,
>                             FDT_IMSIC_INT_CELLS);
>       qemu_fdt_setprop(ms->fdt, imsic_name, "interrupt-controller", NULL, 0);
>       qemu_fdt_setprop(ms->fdt, imsic_name, "msi-controller", NULL, 0);
> -    qemu_fdt_setprop_cell(ms->fdt, imsic_name, "#msi-cells", 0);
>       qemu_fdt_setprop(ms->fdt, imsic_name, "interrupts-extended",
>                        imsic_cells, ms->smp.cpus * sizeof(uint32_t) * 2);
>       qemu_fdt_setprop(ms->fdt, imsic_name, "reg", imsic_regs,
Re: [PATCH for-9.1 1/2] Revert "hw/riscv/virt.c: imsics DT: add '#msi-cells'"
Posted by Philippe Mathieu-Daudé 3 months, 1 week ago
On 16/8/24 18:07, Andrew Jones wrote:
> This reverts commit f42cdf2ea5b3a1dc369792d7acbf9cd3e5c90815.
> 
> Linux does not properly handle '#msi-cells=<0>' when searching for
> MSI controllers for PCI devices which results in the devices being
> unable to use MSIs. A patch for Linux has been sent[1] but until it,
> or something like it, is merged and in distro kernels we should stop
> adding the property. It's harmless to stop adding it since the
> absence of the property and a value of zero for the property mean
> the same thing according to the DT binding definition.
> 

This reverts commit f42cdf2ea5b3a1dc369792d7acbf9cd3e5c90815.

> Link: https://lore.kernel.org/all/20240816124957.130017-2-ajones@ventanamicro.com/ # 1
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> ---
>   hw/riscv/virt.c | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> index 9981e0f6c9b9..cef41c150aaf 100644
> --- a/hw/riscv/virt.c
> +++ b/hw/riscv/virt.c
> @@ -552,7 +552,6 @@ static void create_fdt_one_imsic(RISCVVirtState *s, hwaddr base_addr,
>                             FDT_IMSIC_INT_CELLS);
>       qemu_fdt_setprop(ms->fdt, imsic_name, "interrupt-controller", NULL, 0);
>       qemu_fdt_setprop(ms->fdt, imsic_name, "msi-controller", NULL, 0);
> -    qemu_fdt_setprop_cell(ms->fdt, imsic_name, "#msi-cells", 0);
>       qemu_fdt_setprop(ms->fdt, imsic_name, "interrupts-extended",
>                        imsic_cells, ms->smp.cpus * sizeof(uint32_t) * 2);
>       qemu_fdt_setprop(ms->fdt, imsic_name, "reg", imsic_regs,
Re: [PATCH for-9.1 1/2] Revert "hw/riscv/virt.c: imsics DT: add '#msi-cells'"
Posted by Philippe Mathieu-Daudé 3 months, 1 week ago
On 16/8/24 18:27, Philippe Mathieu-Daudé wrote:
> On 16/8/24 18:07, Andrew Jones wrote:
>> This reverts commit f42cdf2ea5b3a1dc369792d7acbf9cd3e5c90815.

Ahah sorry I'm not seeing well after a long day in front of the
monitor =)

>> Linux does not properly handle '#msi-cells=<0>' when searching for
>> MSI controllers for PCI devices which results in the devices being
>> unable to use MSIs. A patch for Linux has been sent[1] but until it,
>> or something like it, is merged and in distro kernels we should stop
>> adding the property. It's harmless to stop adding it since the
>> absence of the property and a value of zero for the property mean
>> the same thing according to the DT binding definition.
>>
> 
> This reverts commit f42cdf2ea5b3a1dc369792d7acbf9cd3e5c90815.
> 
>> Link: 
>> https://lore.kernel.org/all/20240816124957.130017-2-ajones@ventanamicro.com/ # 1
>> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
>> ---
>>   hw/riscv/virt.c | 1 -
>>   1 file changed, 1 deletion(-)
>>
>> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
>> index 9981e0f6c9b9..cef41c150aaf 100644
>> --- a/hw/riscv/virt.c
>> +++ b/hw/riscv/virt.c
>> @@ -552,7 +552,6 @@ static void create_fdt_one_imsic(RISCVVirtState 
>> *s, hwaddr base_addr,
>>                             FDT_IMSIC_INT_CELLS);
>>       qemu_fdt_setprop(ms->fdt, imsic_name, "interrupt-controller", 
>> NULL, 0);
>>       qemu_fdt_setprop(ms->fdt, imsic_name, "msi-controller", NULL, 0);
>> -    qemu_fdt_setprop_cell(ms->fdt, imsic_name, "#msi-cells", 0);
>>       qemu_fdt_setprop(ms->fdt, imsic_name, "interrupts-extended",
>>                        imsic_cells, ms->smp.cpus * sizeof(uint32_t) * 2);
>>       qemu_fdt_setprop(ms->fdt, imsic_name, "reg", imsic_regs,
>