[PATCH] hw/arm/virt-acpi-build: Fix GSIV values of the {GERR, Sync} interrupts

[PATCH] hw/arm/virt-acpi-build: Fix GSIV values of the {GERR, Sync} interrupts

[Zenghui Yu]

The GSIV values in SMMUv3 IORT node are not correct as they don't match
the SMMUIrq enumeration, which describes the IRQ<->PIN mapping used by
our emulated vSMMU.

Fixes: a703b4f6c1ee ("hw/arm/virt-acpi-build: Add smmuv3 node in IORT table")
Signed-off-by: Zenghui Yu <yuzenghui@huawei.com>
---
 hw/arm/virt-acpi-build.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index f5a2b2d4cb..60fe2e65a7 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -292,8 +292,8 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
         smmu->flags = cpu_to_le32(ACPI_IORT_SMMU_V3_COHACC_OVERRIDE);
         smmu->event_gsiv = cpu_to_le32(irq);
         smmu->pri_gsiv = cpu_to_le32(irq + 1);
-        smmu->gerr_gsiv = cpu_to_le32(irq + 2);
-        smmu->sync_gsiv = cpu_to_le32(irq + 3);
+        smmu->sync_gsiv = cpu_to_le32(irq + 2);
+        smmu->gerr_gsiv = cpu_to_le32(irq + 3);
 
         /* Identity RID mapping covering the whole input RID range */
         idmap = &smmu->id_mapping_array[0];
-- 
2.19.1

Re: [PATCH] hw/arm/virt-acpi-build: Fix GSIV values of the {GERR, Sync} interrupts

[Auger Eric]

Hi Zenghui,

On 4/2/21 10:47 AM, Zenghui Yu wrote:
> The GSIV values in SMMUv3 IORT node are not correct as they don't match
> the SMMUIrq enumeration, which describes the IRQ<->PIN mapping used by
> our emulated vSMMU.
> 
> Fixes: a703b4f6c1ee ("hw/arm/virt-acpi-build: Add smmuv3 node in IORT table")
> Signed-off-by: Zenghui Yu <yuzenghui@huawei.com>
Acked-by: Eric Auger <eric.auger@redhat.com>

Thanks!

Eric
> ---
>  hw/arm/virt-acpi-build.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index f5a2b2d4cb..60fe2e65a7 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -292,8 +292,8 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>          smmu->flags = cpu_to_le32(ACPI_IORT_SMMU_V3_COHACC_OVERRIDE);
>          smmu->event_gsiv = cpu_to_le32(irq);
>          smmu->pri_gsiv = cpu_to_le32(irq + 1);
> -        smmu->gerr_gsiv = cpu_to_le32(irq + 2);
> -        smmu->sync_gsiv = cpu_to_le32(irq + 3);
> +        smmu->sync_gsiv = cpu_to_le32(irq + 2);
> +        smmu->gerr_gsiv = cpu_to_le32(irq + 3);
>  
>          /* Identity RID mapping covering the whole input RID range */
>          idmap = &smmu->id_mapping_array[0];
> 

Re: [PATCH] hw/arm/virt-acpi-build: Fix GSIV values of the {GERR, Sync} interrupts

[Peter Maydell]

On Tue, 6 Apr 2021 at 11:10, Auger Eric <eric.auger@redhat.com> wrote:
>
> Hi Zenghui,
>
> On 4/2/21 10:47 AM, Zenghui Yu wrote:
> > The GSIV values in SMMUv3 IORT node are not correct as they don't match
> > the SMMUIrq enumeration, which describes the IRQ<->PIN mapping used by
> > our emulated vSMMU.
> >
> > Fixes: a703b4f6c1ee ("hw/arm/virt-acpi-build: Add smmuv3 node in IORT table")
> > Signed-off-by: Zenghui Yu <yuzenghui@huawei.com>
> Acked-by: Eric Auger <eric.auger@redhat.com>

Eric, when you send an acked-by tag do you mean to say that you've
reviewed the patch, or merely that you think it's basically the
right thing but you haven't actually looked at the details?

(I ask because if the former I can just put this in target-arm.next,
but if the latter then I need to dig out the SMMU spec and review
the patch myself :-))

thanks
-- PMM

Re: [PATCH] hw/arm/virt-acpi-build: Fix GSIV values of the {GERR, Sync} interrupts

[Auger Eric]

Hi Peter,

On 4/6/21 12:44 PM, Peter Maydell wrote:
> On Tue, 6 Apr 2021 at 11:10, Auger Eric <eric.auger@redhat.com> wrote:
>>
>> Hi Zenghui,
>>
>> On 4/2/21 10:47 AM, Zenghui Yu wrote:
>>> The GSIV values in SMMUv3 IORT node are not correct as they don't match
>>> the SMMUIrq enumeration, which describes the IRQ<->PIN mapping used by
>>> our emulated vSMMU.
>>>
>>> Fixes: a703b4f6c1ee ("hw/arm/virt-acpi-build: Add smmuv3 node in IORT table")
>>> Signed-off-by: Zenghui Yu <yuzenghui@huawei.com>
>> Acked-by: Eric Auger <eric.auger@redhat.com>
> 
> Eric, when you send an acked-by tag do you mean to say that you've
> reviewed the patch, or merely that you think it's basically the
> right thing but you haven't actually looked at the details?

I mean I have reviewed the patch carefully and I think it is good to go.
I thought that as a maintainer for the arm smmu component I was supposed
to send an A-b instead of an R-b.
> 
> (I ask because if the former I can just put this in target-arm.next,
> but if the latter then I need to dig out the SMMU spec and review
> the patch myself :-))

Yes that's rather the former but obviously if you have some cycles /
interest in the topic I am more than happy to get your opinion too!

Thanks

Eric
> 
> thanks
> -- PMM
> 

Re: [PATCH] hw/arm/virt-acpi-build: Fix GSIV values of the {GERR, Sync} interrupts

[Peter Maydell]

On Tue, 6 Apr 2021 at 13:23, Auger Eric <eric.auger@redhat.com> wrote:
>
> Hi Peter,
>
> On 4/6/21 12:44 PM, Peter Maydell wrote:
> > On Tue, 6 Apr 2021 at 11:10, Auger Eric <eric.auger@redhat.com> wrote:
> >>
> >> Hi Zenghui,
> >>
> >> On 4/2/21 10:47 AM, Zenghui Yu wrote:
> >>> The GSIV values in SMMUv3 IORT node are not correct as they don't match
> >>> the SMMUIrq enumeration, which describes the IRQ<->PIN mapping used by
> >>> our emulated vSMMU.
> >>>
> >>> Fixes: a703b4f6c1ee ("hw/arm/virt-acpi-build: Add smmuv3 node in IORT table")
> >>> Signed-off-by: Zenghui Yu <yuzenghui@huawei.com>
> >> Acked-by: Eric Auger <eric.auger@redhat.com>
> >
> > Eric, when you send an acked-by tag do you mean to say that you've
> > reviewed the patch, or merely that you think it's basically the
> > right thing but you haven't actually looked at the details?
>
> I mean I have reviewed the patch carefully and I think it is good to go.
> I thought that as a maintainer for the arm smmu component I was supposed
> to send an A-b instead of an R-b.

The usual meaning I think is that "Acked-by" means "I'm the maintainer,
I've seen this going by, and I'm basically OK with this" (ie it's you
saying "I'm not NAKing it") -- so it's not as "strong" as a "Reviewed-by"
tag (which means "I've reviewed it").

thanks
-- PMM

Re: [PATCH] hw/arm/virt-acpi-build: Fix GSIV values of the {GERR, Sync} interrupts

[Auger Eric]

Hi Peter,

On 4/6/21 2:31 PM, Peter Maydell wrote:
> On Tue, 6 Apr 2021 at 13:23, Auger Eric <eric.auger@redhat.com> wrote:
>>
>> Hi Peter,
>>
>> On 4/6/21 12:44 PM, Peter Maydell wrote:
>>> On Tue, 6 Apr 2021 at 11:10, Auger Eric <eric.auger@redhat.com> wrote:
>>>>
>>>> Hi Zenghui,
>>>>
>>>> On 4/2/21 10:47 AM, Zenghui Yu wrote:
>>>>> The GSIV values in SMMUv3 IORT node are not correct as they don't match
>>>>> the SMMUIrq enumeration, which describes the IRQ<->PIN mapping used by
>>>>> our emulated vSMMU.
>>>>>
>>>>> Fixes: a703b4f6c1ee ("hw/arm/virt-acpi-build: Add smmuv3 node in IORT table")
>>>>> Signed-off-by: Zenghui Yu <yuzenghui@huawei.com>
>>>> Acked-by: Eric Auger <eric.auger@redhat.com>
>>>
>>> Eric, when you send an acked-by tag do you mean to say that you've
>>> reviewed the patch, or merely that you think it's basically the
>>> right thing but you haven't actually looked at the details?
>>
>> I mean I have reviewed the patch carefully and I think it is good to go.
>> I thought that as a maintainer for the arm smmu component I was supposed
>> to send an A-b instead of an R-b.
> 
> The usual meaning I think is that "Acked-by" means "I'm the maintainer,
> I've seen this going by, and I'm basically OK with this" (ie it's you
> saying "I'm not NAKing it") -- so it's not as "strong" as a "Reviewed-by"
> tag (which means "I've reviewed it").

Hum OK, I thought it was stronger than the R-b. So in the future, wrt
the SMMU component, I will give an R-b as I always do a proper review.

Thanks

Eric
> 
> thanks
> -- PMM
> 

Re: [PATCH] hw/arm/virt-acpi-build: Fix GSIV values of the {GERR, Sync} interrupts

[Peter Maydell]

On Fri, 2 Apr 2021 at 09:48, Zenghui Yu <yuzenghui@huawei.com> wrote:
>
> The GSIV values in SMMUv3 IORT node are not correct as they don't match
> the SMMUIrq enumeration, which describes the IRQ<->PIN mapping used by
> our emulated vSMMU.
>
> Fixes: a703b4f6c1ee ("hw/arm/virt-acpi-build: Add smmuv3 node in IORT table")
> Signed-off-by: Zenghui Yu <yuzenghui@huawei.com>
> ---
>  hw/arm/virt-acpi-build.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index f5a2b2d4cb..60fe2e65a7 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -292,8 +292,8 @@ build_iort(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>          smmu->flags = cpu_to_le32(ACPI_IORT_SMMU_V3_COHACC_OVERRIDE);
>          smmu->event_gsiv = cpu_to_le32(irq);
>          smmu->pri_gsiv = cpu_to_le32(irq + 1);
> -        smmu->gerr_gsiv = cpu_to_le32(irq + 2);
> -        smmu->sync_gsiv = cpu_to_le32(irq + 3);
> +        smmu->sync_gsiv = cpu_to_le32(irq + 2);
> +        smmu->gerr_gsiv = cpu_to_le32(irq + 3);
>
>          /* Identity RID mapping covering the whole input RID range */
>          idmap = &smmu->id_mapping_array[0];
> --



Applied to target-arm.next, thanks.

-- PMM