| Message ID | 20200925133902.28349-1-graeme@nuviainc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/arm/sbsa-ref : Fix SMMUv3 Initialisation | expand |
Hi Gregory, On 9/25/20 3:39 PM, Graeme Gregory wrote: > SMMUv3 has an error in previous patch where a i was transposed to a 1 > meaning interrupts would not have been correctly assigned to the SMMUv3 > instance. > > The code also contained an error in that the IRQs were never allocated > in the irqmap. > > Fixes: 48ba18e6d3f3 ("hw/arm/sbsa-ref: Simplify by moving the gic in the machine state") > Signed-off-by: Graeme Gregory <graeme@nuviainc.com> Reviewed-by: Eric Auger <eric.auger@redhat.com> Thanks Eric > --- > hw/arm/sbsa-ref.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c > index 257ada9425..9109fb58be 100644 > --- a/hw/arm/sbsa-ref.c > +++ b/hw/arm/sbsa-ref.c > @@ -138,6 +138,7 @@ static const int sbsa_ref_irqmap[] = { > [SBSA_SECURE_UART_MM] = 9, > [SBSA_AHCI] = 10, > [SBSA_EHCI] = 11, > + [SBSA_SMMU] = 12, /* ... to 15 */ > }; > > static uint64_t sbsa_ref_cpu_mp_affinity(SBSAMachineState *sms, int idx) > @@ -530,7 +531,7 @@ static void create_smmu(const SBSAMachineState *sms, PCIBus *bus) > sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base); > for (i = 0; i < NUM_SMMU_IRQS; i++) { > sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, > - qdev_get_gpio_in(sms->gic, irq + 1)); > + qdev_get_gpio_in(sms->gic, irq + i)); > } > } > >
Hi Gregory, On 9/25/20 4:00 PM, Auger Eric wrote: > Hi Gregory, > > On 9/25/20 3:39 PM, Graeme Gregory wrote: >> SMMUv3 has an error in previous patch where a i was transposed to a 1 >> meaning interrupts would not have been correctly assigned to the SMMUv3 >> instance. This is a first issue, fixing 48ba18e6d3f3. >> >> The code also contained an error in that the IRQs were never allocated >> in the irqmap. This is another issue, not well explained. IIUC IRQs *are* allocated as IRQ #0, right? This fixes commit e9fdf453240 ("hw/arm: Add arm SBSA reference machine, devices part"). Can you split this in another patch please? Eventually Cc'ing qemu-stable@nongnu.org as suggested by Peter. >> >> Fixes: 48ba18e6d3f3 ("hw/arm/sbsa-ref: Simplify by moving the gic in the machine state") >> Signed-off-by: Graeme Gregory <graeme@nuviainc.com> > Reviewed-by: Eric Auger <eric.auger@redhat.com> > > Thanks > > Eric > >> --- >> hw/arm/sbsa-ref.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c >> index 257ada9425..9109fb58be 100644 >> --- a/hw/arm/sbsa-ref.c >> +++ b/hw/arm/sbsa-ref.c >> @@ -138,6 +138,7 @@ static const int sbsa_ref_irqmap[] = { >> [SBSA_SECURE_UART_MM] = 9, >> [SBSA_AHCI] = 10, >> [SBSA_EHCI] = 11, >> + [SBSA_SMMU] = 12, /* ... to 15 */ >> }; >> >> static uint64_t sbsa_ref_cpu_mp_affinity(SBSAMachineState *sms, int idx) >> @@ -530,7 +531,7 @@ static void create_smmu(const SBSAMachineState *sms, PCIBus *bus) >> sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base); >> for (i = 0; i < NUM_SMMU_IRQS; i++) { >> sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, >> - qdev_get_gpio_in(sms->gic, irq + 1)); >> + qdev_get_gpio_in(sms->gic, irq + i)); BTW this fix is already in Peter's queue: https://www.mail-archive.com/qemu-devel@nongnu.org/msg732819.html Thanks, Phil. >> } >> } >> >> >
On Sat, Sep 26, 2020 at 09:45:24AM +0200, Philippe Mathieu-Daudé wrote: > Hi Gregory, > > On 9/25/20 4:00 PM, Auger Eric wrote: > > Hi Gregory, > > > > On 9/25/20 3:39 PM, Graeme Gregory wrote: > >> SMMUv3 has an error in previous patch where a i was transposed to a 1 > >> meaning interrupts would not have been correctly assigned to the SMMUv3 > >> instance. > > This is a first issue, fixing 48ba18e6d3f3. > > >> > >> The code also contained an error in that the IRQs were never allocated > >> in the irqmap. > > This is another issue, not well explained. IIUC IRQs *are* allocated as > IRQ #0, right? > > This fixes commit e9fdf453240 ("hw/arm: Add arm SBSA reference machine, > devices part"). Can you split this in another patch please? Eventually > Cc'ing qemu-stable@nongnu.org as suggested by Peter. > Ok I will split and issue v2 ASAP Thanks Graeme > >> > >> Fixes: 48ba18e6d3f3 ("hw/arm/sbsa-ref: Simplify by moving the gic in the machine state") > >> Signed-off-by: Graeme Gregory <graeme@nuviainc.com> > > Reviewed-by: Eric Auger <eric.auger@redhat.com> > > > > Thanks > > > > Eric > > > >> --- > >> hw/arm/sbsa-ref.c | 3 ++- > >> 1 file changed, 2 insertions(+), 1 deletion(-) > >> > >> diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c > >> index 257ada9425..9109fb58be 100644 > >> --- a/hw/arm/sbsa-ref.c > >> +++ b/hw/arm/sbsa-ref.c > >> @@ -138,6 +138,7 @@ static const int sbsa_ref_irqmap[] = { > >> [SBSA_SECURE_UART_MM] = 9, > >> [SBSA_AHCI] = 10, > >> [SBSA_EHCI] = 11, > >> + [SBSA_SMMU] = 12, /* ... to 15 */ > >> }; > >> > >> static uint64_t sbsa_ref_cpu_mp_affinity(SBSAMachineState *sms, int idx) > >> @@ -530,7 +531,7 @@ static void create_smmu(const SBSAMachineState *sms, PCIBus *bus) > >> sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base); > >> for (i = 0; i < NUM_SMMU_IRQS; i++) { > >> sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, > >> - qdev_get_gpio_in(sms->gic, irq + 1)); > >> + qdev_get_gpio_in(sms->gic, irq + i)); > > BTW this fix is already in Peter's queue: > https://www.mail-archive.com/qemu-devel@nongnu.org/msg732819.html > > Thanks, > > Phil. > > >> } > >> } > >> > >> > > >
diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c index 257ada9425..9109fb58be 100644 --- a/hw/arm/sbsa-ref.c +++ b/hw/arm/sbsa-ref.c @@ -138,6 +138,7 @@ static const int sbsa_ref_irqmap[] = { [SBSA_SECURE_UART_MM] = 9, [SBSA_AHCI] = 10, [SBSA_EHCI] = 11, + [SBSA_SMMU] = 12, /* ... to 15 */ }; static uint64_t sbsa_ref_cpu_mp_affinity(SBSAMachineState *sms, int idx) @@ -530,7 +531,7 @@ static void create_smmu(const SBSAMachineState *sms, PCIBus *bus) sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base); for (i = 0; i < NUM_SMMU_IRQS; i++) { sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, - qdev_get_gpio_in(sms->gic, irq + 1)); + qdev_get_gpio_in(sms->gic, irq + i)); } }
SMMUv3 has an error in previous patch where a i was transposed to a 1 meaning interrupts would not have been correctly assigned to the SMMUv3 instance. The code also contained an error in that the IRQs were never allocated in the irqmap. Fixes: 48ba18e6d3f3 ("hw/arm/sbsa-ref: Simplify by moving the gic in the machine state") Signed-off-by: Graeme Gregory <graeme@nuviainc.com> --- hw/arm/sbsa-ref.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)