Accelerated SMMUv3 instances rely on the physical SMMUv3 for nested
translation (Guest Stage-1, Host Stage-2). In this mode the guest’s
Stage-1 tables are programmed directly into hardware, and QEMU should
not attempt to walk them for translation since doing so is not reliably
safe. For vfio-pci endpoints behind such a vSMMU, the only translation
QEMU is responsible for is the MSI doorbell used during KVM MSI setup.

Add a device property to carry the MSI doorbell GPA from the virt
machine, and expose it through a new get_msi_direct_gpa PCIIOMMUOp.
kvm_arch_fixup_msi_route() can then use this GPA directly instead of
attempting a software walk of guest translation tables.

This enables correct MSI routing with accelerated SMMUv3 while avoiding
unsafe accesses to page tables.

For meaningful use of vfio-pci devices with accelerated SMMUv3, both KVM
and a kernel irqchip are required. Enforce this requirement when accel=on
is selected.

Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
---
 hw/arm/smmuv3-accel.c   | 10 ++++++++++
 hw/arm/smmuv3.c         |  2 ++
 hw/arm/virt.c           | 22 ++++++++++++++++++++++
 include/hw/arm/smmuv3.h |  1 +
 4 files changed, 35 insertions(+)

diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
index 65b577f49a..8f7c0cda05 100644
--- a/hw/arm/smmuv3-accel.c
+++ b/hw/arm/smmuv3-accel.c
@@ -392,6 +392,15 @@ static void smmuv3_accel_unset_iommu_device(PCIBus *bus, void *opaque,
     }
 }
 
+static uint64_t smmuv3_accel_get_msi_gpa(PCIBus *bus, void *opaque, int devfn)
+{
+    SMMUState *bs = opaque;
+    SMMUv3State *s = ARM_SMMUV3(bs);
+
+    g_assert(s->msi_gpa);
+    return s->msi_gpa;
+}
+
 /*
  * Only allow PCIe bridges, pxb-pcie roots, and GPEX roots so vfio-pci
  * endpoints can sit downstream. Accelerated SMMUv3 requires a vfio-pci
@@ -496,6 +505,7 @@ static const PCIIOMMUOps smmuv3_accel_ops = {
     .get_viommu_flags = smmuv3_accel_get_viommu_flags,
     .set_iommu_device = smmuv3_accel_set_iommu_device,
     .unset_iommu_device = smmuv3_accel_unset_iommu_device,
+    .get_msi_direct_gpa = smmuv3_accel_get_msi_gpa,
 };
 
 /* Based on SMUUv3 GPBA.ABORT configuration, attach a corresponding HWPT */
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index 42c60b1ec8..f02e3ee46c 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -1998,6 +1998,8 @@ static const Property smmuv3_properties[] = {
      * Defaults to stage 1
      */
     DEFINE_PROP_STRING("stage", SMMUv3State, stage),
+    /* GPA of MSI doorbell, for SMMUv3 accel use. */
+    DEFINE_PROP_UINT64("msi-gpa", SMMUv3State, msi_gpa, 0),
 };
 
 static void smmuv3_instance_init(Object *obj)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 25fb2bab56..ea3231543a 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -3052,6 +3052,14 @@ static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
             /* The new SMMUv3 device is specific to the PCI bus */
             object_property_set_bool(OBJECT(dev), "smmu_per_bus", true, NULL);
         }
+        if (object_property_find(OBJECT(dev), "accel") &&
+            object_property_get_bool(OBJECT(dev), "accel", &error_abort)) {
+            if (!kvm_enabled() || !kvm_irqchip_in_kernel()) {
+                error_setg(errp, "SMMUv3 accel=on requires KVM with "
+                           "kernel-irqchip=on support");
+                    return;
+            }
+        }
     }
 }
 
@@ -3088,6 +3096,20 @@ static void virt_machine_device_plug_cb(HotplugHandler *hotplug_dev,
             }
 
             create_smmuv3_dev_dtb(vms, dev, bus);
+            if (object_property_find(OBJECT(dev), "accel") &&
+                object_property_get_bool(OBJECT(dev), "accel", &error_abort)) {
+                hwaddr db_start;
+
+                if (vms->msi_controller == VIRT_MSI_CTRL_ITS) {
+                    /* GITS_TRANSLATER page + offset */
+                    db_start = base_memmap[VIRT_GIC_ITS].base + 0x10000 + 0x40;
+                } else {
+                    /* MSI_SETSPI_NS page + offset */
+                    db_start = base_memmap[VIRT_GIC_V2M].base + 0x40;
+                }
+                object_property_set_uint(OBJECT(dev), "msi-gpa", db_start,
+                                         &error_abort);
+            }
         }
     }
 
diff --git a/include/hw/arm/smmuv3.h b/include/hw/arm/smmuv3.h
index e54ece2d38..5616a8a2be 100644
--- a/include/hw/arm/smmuv3.h
+++ b/include/hw/arm/smmuv3.h
@@ -67,6 +67,7 @@ struct SMMUv3State {
     /* SMMU has HW accelerator support for nested S1 + s2 */
     bool accel;
     struct SMMUv3AccelState *s_accel;
+    uint64_t msi_gpa;
 };
 
 typedef enum {
-- 
2.43.0

On 11/20/25 14:21, Shameer Kolothum wrote:
> Accelerated SMMUv3 instances rely on the physical SMMUv3 for nested
> translation (Guest Stage-1, Host Stage-2). In this mode the guest’s
> Stage-1 tables are programmed directly into hardware, and QEMU should
> not attempt to walk them for translation since doing so is not reliably
> safe. For vfio-pci endpoints behind such a vSMMU, the only translation
> QEMU is responsible for is the MSI doorbell used during KVM MSI setup.
> 
> Add a device property to carry the MSI doorbell GPA from the virt
> machine, and expose it through a new get_msi_direct_gpa PCIIOMMUOp.
> kvm_arch_fixup_msi_route() can then use this GPA directly instead of
> attempting a software walk of guest translation tables.
> 
> This enables correct MSI routing with accelerated SMMUv3 while avoiding
> unsafe accesses to page tables.
> 
> For meaningful use of vfio-pci devices with accelerated SMMUv3, both KVM
> and a kernel irqchip are required. Enforce this requirement when accel=on
> is selected.

There are multiple changes in one patch. I suggest splitting.

> 
> Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
> ---
>   hw/arm/smmuv3-accel.c   | 10 ++++++++++
>   hw/arm/smmuv3.c         |  2 ++
>   hw/arm/virt.c           | 22 ++++++++++++++++++++++
>   include/hw/arm/smmuv3.h |  1 +
>   4 files changed, 35 insertions(+)
> 
> diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
> index 65b577f49a..8f7c0cda05 100644
> --- a/hw/arm/smmuv3-accel.c
> +++ b/hw/arm/smmuv3-accel.c
> @@ -392,6 +392,15 @@ static void smmuv3_accel_unset_iommu_device(PCIBus *bus, void *opaque,
>       }
>   }
>   
> +static uint64_t smmuv3_accel_get_msi_gpa(PCIBus *bus, void *opaque, int devfn)
> +{
> +    SMMUState *bs = opaque;
> +    SMMUv3State *s = ARM_SMMUV3(bs);
> +
> +    g_assert(s->msi_gpa);
> +    return s->msi_gpa;
> +}
> +
>   /*
>    * Only allow PCIe bridges, pxb-pcie roots, and GPEX roots so vfio-pci
>    * endpoints can sit downstream. Accelerated SMMUv3 requires a vfio-pci
> @@ -496,6 +505,7 @@ static const PCIIOMMUOps smmuv3_accel_ops = {
>       .get_viommu_flags = smmuv3_accel_get_viommu_flags,
>       .set_iommu_device = smmuv3_accel_set_iommu_device,
>       .unset_iommu_device = smmuv3_accel_unset_iommu_device,
> +    .get_msi_direct_gpa = smmuv3_accel_get_msi_gpa,
>   };
>   
>   /* Based on SMUUv3 GPBA.ABORT configuration, attach a corresponding HWPT */
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index 42c60b1ec8..f02e3ee46c 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -1998,6 +1998,8 @@ static const Property smmuv3_properties[] = {
>        * Defaults to stage 1
>        */
>       DEFINE_PROP_STRING("stage", SMMUv3State, stage),
> +    /* GPA of MSI doorbell, for SMMUv3 accel use. */
> +    DEFINE_PROP_UINT64("msi-gpa", SMMUv3State, msi_gpa, 0),
>   };
>   
>   static void smmuv3_instance_init(Object *obj)
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 25fb2bab56..ea3231543a 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -3052,6 +3052,14 @@ static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>               /* The new SMMUv3 device is specific to the PCI bus */
>               object_property_set_bool(OBJECT(dev), "smmu_per_bus", true, NULL);
>           }
> +        if (object_property_find(OBJECT(dev), "accel") &&
> +            object_property_get_bool(OBJECT(dev), "accel", &error_abort)) {
> +            if (!kvm_enabled() || !kvm_irqchip_in_kernel()) {
> +                error_setg(errp, "SMMUv3 accel=on requires KVM with "
> +                           "kernel-irqchip=on support");
> +                    return;

Couldn't these checks be done in the "smmuv3-accel" model realize
handler instead ?

  
> +            }
> +        }
>       }
>   }
>   
> @@ -3088,6 +3096,20 @@ static void virt_machine_device_plug_cb(HotplugHandler *hotplug_dev,
>               }
>   
>               create_smmuv3_dev_dtb(vms, dev, bus);
> +            if (object_property_find(OBJECT(dev), "accel") &&
> +                object_property_get_bool(OBJECT(dev), "accel", &error_abort)) {
> +                hwaddr db_start;
> +
> +                if (vms->msi_controller == VIRT_MSI_CTRL_ITS) {
> +                    /* GITS_TRANSLATER page + offset */
> +                    db_start = base_memmap[VIRT_GIC_ITS].base + 0x10000 + 0x40;
> +                } else {
> +                    /* MSI_SETSPI_NS page + offset */
> +                    db_start = base_memmap[VIRT_GIC_V2M].base + 0x40;
> +                }
> +                object_property_set_uint(OBJECT(dev), "msi-gpa", db_start,
> +                                         &error_abort);

IIRC, this plug handler is called after the device realize handler
and setting properties after realize is strongly discouraged.

C.

> +            }
>           }
>       }
>   
> diff --git a/include/hw/arm/smmuv3.h b/include/hw/arm/smmuv3.h
> index e54ece2d38..5616a8a2be 100644
> --- a/include/hw/arm/smmuv3.h
> +++ b/include/hw/arm/smmuv3.h
> @@ -67,6 +67,7 @@ struct SMMUv3State {
>       /* SMMU has HW accelerator support for nested S1 + s2 */
>       bool accel;
>       struct SMMUv3AccelState *s_accel;
> +    uint64_t msi_gpa;
>   };
>   
>   typedef enum {

On Thu, Nov 20, 2025 at 01:21:57PM +0000, Shameer Kolothum wrote:
> Accelerated SMMUv3 instances rely on the physical SMMUv3 for nested
> translation (Guest Stage-1, Host Stage-2). In this mode the guest’s
> Stage-1 tables are programmed directly into hardware, and QEMU should
> not attempt to walk them for translation since doing so is not reliably
> safe. For vfio-pci endpoints behind such a vSMMU, the only translation
> QEMU is responsible for is the MSI doorbell used during KVM MSI setup.
> 
> Add a device property to carry the MSI doorbell GPA from the virt
> machine, and expose it through a new get_msi_direct_gpa PCIIOMMUOp.
> kvm_arch_fixup_msi_route() can then use this GPA directly instead of
> attempting a software walk of guest translation tables.
> 
> This enables correct MSI routing with accelerated SMMUv3 while avoiding
> unsafe accesses to page tables.
> 
> For meaningful use of vfio-pci devices with accelerated SMMUv3, both KVM
> and a kernel irqchip are required. Enforce this requirement when accel=on
> is selected.
> 
> Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>

Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>

Nits:

> +++ b/hw/arm/virt.c
> @@ -3052,6 +3052,14 @@ static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>              /* The new SMMUv3 device is specific to the PCI bus */
>              object_property_set_bool(OBJECT(dev), "smmu_per_bus", true, NULL);
>          }
> +        if (object_property_find(OBJECT(dev), "accel") &&
> +            object_property_get_bool(OBJECT(dev), "accel", &error_abort)) {

Do we need object_property_find()? A later patch seems to drop it.
Perhaps we shouldn't add it in the first place?


> @@ -3088,6 +3096,20 @@ static void virt_machine_device_plug_cb(HotplugHandler *hotplug_dev,
>              }
>  
>              create_smmuv3_dev_dtb(vms, dev, bus);
> +            if (object_property_find(OBJECT(dev), "accel") &&
> +                object_property_get_bool(OBJECT(dev), "accel", &error_abort)) {

Ditto

> -----Original Message-----
> From: Nicolin Chen <nicolinc@nvidia.com>
> Sent: 20 November 2025 21:22
> To: Shameer Kolothum <skolothumtho@nvidia.com>
> Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org;
> eric.auger@redhat.com; peter.maydell@linaro.org; Jason Gunthorpe
> <jgg@nvidia.com>; ddutile@redhat.com; berrange@redhat.com; Nathan
> Chen <nathanc@nvidia.com>; Matt Ochs <mochs@nvidia.com>;
> smostafa@google.com; wangzhou1@hisilicon.com;
> jiangkunkun@huawei.com; jonathan.cameron@huawei.com;
> zhangfei.gao@linaro.org; zhenzhong.duan@intel.com; yi.l.liu@intel.com;
> Krishnakant Jaju <kjaju@nvidia.com>
> Subject: Re: [PATCH v6 17/33] hw/arm/smmuv3: Add support for providing a
> direct MSI doorbell GPA
> 
> On Thu, Nov 20, 2025 at 01:21:57PM +0000, Shameer Kolothum wrote:
> > Accelerated SMMUv3 instances rely on the physical SMMUv3 for nested
> > translation (Guest Stage-1, Host Stage-2). In this mode the guest’s
> > Stage-1 tables are programmed directly into hardware, and QEMU should
> > not attempt to walk them for translation since doing so is not reliably
> > safe. For vfio-pci endpoints behind such a vSMMU, the only translation
> > QEMU is responsible for is the MSI doorbell used during KVM MSI setup.
> >
> > Add a device property to carry the MSI doorbell GPA from the virt
> > machine, and expose it through a new get_msi_direct_gpa PCIIOMMUOp.
> > kvm_arch_fixup_msi_route() can then use this GPA directly instead of
> > attempting a software walk of guest translation tables.
> >
> > This enables correct MSI routing with accelerated SMMUv3 while avoiding
> > unsafe accesses to page tables.
> >
> > For meaningful use of vfio-pci devices with accelerated SMMUv3, both KVM
> > and a kernel irqchip are required. Enforce this requirement when accel=on
> > is selected.
> >
> > Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
> 
> Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
> 
> Nits:
> 
> > +++ b/hw/arm/virt.c
> > @@ -3052,6 +3052,14 @@ static void
> virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
> >              /* The new SMMUv3 device is specific to the PCI bus */
> >              object_property_set_bool(OBJECT(dev), "smmu_per_bus", true,
> NULL);
> >          }
> > +        if (object_property_find(OBJECT(dev), "accel") &&
> > +            object_property_get_bool(OBJECT(dev), "accel", &error_abort)) {
> 
> Do we need object_property_find()? A later patch seems to drop it.
> Perhaps we shouldn't add it in the first place?

We need that at this stage as we haven't added the "accel" property yet
and that will cause "make check" tests to fail without that.

We remove it once we introduce "accel" property later.

Thanks,
Shameer

On Fri, Nov 21, 2025 at 01:57:37AM -0800, Shameer Kolothum wrote:
> > On Thu, Nov 20, 2025 at 01:21:57PM +0000, Shameer Kolothum wrote:
> > > Accelerated SMMUv3 instances rely on the physical SMMUv3 for nested
> > > translation (Guest Stage-1, Host Stage-2). In this mode the guest’s
> > > Stage-1 tables are programmed directly into hardware, and QEMU should
> > > not attempt to walk them for translation since doing so is not reliably
> > > safe. For vfio-pci endpoints behind such a vSMMU, the only translation
> > > QEMU is responsible for is the MSI doorbell used during KVM MSI setup.
> > >
> > > Add a device property to carry the MSI doorbell GPA from the virt
> > > machine, and expose it through a new get_msi_direct_gpa PCIIOMMUOp.
> > > kvm_arch_fixup_msi_route() can then use this GPA directly instead of
> > > attempting a software walk of guest translation tables.
> > >
> > > This enables correct MSI routing with accelerated SMMUv3 while avoiding
> > > unsafe accesses to page tables.
> > >
> > > For meaningful use of vfio-pci devices with accelerated SMMUv3, both KVM
> > > and a kernel irqchip are required. Enforce this requirement when accel=on
> > > is selected.
> > >
> > > Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
> > 
> > Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
> > 
> > Nits:
> > 
> > > +++ b/hw/arm/virt.c
> > > @@ -3052,6 +3052,14 @@ static void
> > virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
> > >              /* The new SMMUv3 device is specific to the PCI bus */
> > >              object_property_set_bool(OBJECT(dev), "smmu_per_bus", true,
> > NULL);
> > >          }
> > > +        if (object_property_find(OBJECT(dev), "accel") &&
> > > +            object_property_get_bool(OBJECT(dev), "accel", &error_abort)) {
> > 
> > Do we need object_property_find()? A later patch seems to drop it.
> > Perhaps we shouldn't add it in the first place?
> 
> We need that at this stage as we haven't added the "accel" property yet
> and that will cause "make check" tests to fail without that.
>
> We remove it once we introduce "accel" property later.

Hmm, I assume object_property_get_bool() would return false when
"accel" is not available yet? No?

Nicolin

> -----Original Message-----
> From: Nicolin Chen <nicolinc@nvidia.com>
> Sent: 21 November 2025 17:56
> To: Shameer Kolothum <skolothumtho@nvidia.com>
> Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org;
> eric.auger@redhat.com; peter.maydell@linaro.org; Jason Gunthorpe
> <jgg@nvidia.com>; ddutile@redhat.com; berrange@redhat.com; Nathan
> Chen <nathanc@nvidia.com>; Matt Ochs <mochs@nvidia.com>;
> smostafa@google.com; wangzhou1@hisilicon.com;
> jiangkunkun@huawei.com; jonathan.cameron@huawei.com;
> zhangfei.gao@linaro.org; zhenzhong.duan@intel.com; yi.l.liu@intel.com;
> Krishnakant Jaju <kjaju@nvidia.com>
> Subject: Re: [PATCH v6 17/33] hw/arm/smmuv3: Add support for providing a
> direct MSI doorbell GPA
> 


> > > > +        if (object_property_find(OBJECT(dev), "accel") &&
> > > > +            object_property_get_bool(OBJECT(dev), "accel", &error_abort)) {
> > >
> > > Do we need object_property_find()? A later patch seems to drop it.
> > > Perhaps we shouldn't add it in the first place?
> >
> > We need that at this stage as we haven't added the "accel" property yet
> > and that will cause "make check" tests to fail without that.
> >
> > We remove it once we introduce "accel" property later.
> 
> Hmm, I assume object_property_get_bool() would return false when
> "accel" is not available yet? No?

It is the errp that will be set if there is no "accel" is available that will fail the
tests. Another way to avoid object_property_find() is to pass NULL for errp in
object_property_get_bool(), but I think again when we introduce "accel" we have
change that into either errp/error_abort here.

I am not sure there is any other way to handle this.

Thanks,
Shameer

On Mon, Nov 24, 2025 at 12:05:38AM -0800, Shameer Kolothum wrote:
> > > > > +        if (object_property_find(OBJECT(dev), "accel") &&
> > > > > +            object_property_get_bool(OBJECT(dev), "accel", &error_abort)) {
> > > >
> > > > Do we need object_property_find()? A later patch seems to drop it.
> > > > Perhaps we shouldn't add it in the first place?
> > >
> > > We need that at this stage as we haven't added the "accel" property yet
> > > and that will cause "make check" tests to fail without that.
> > >
> > > We remove it once we introduce "accel" property later.
> > 
> > Hmm, I assume object_property_get_bool() would return false when
> > "accel" is not available yet? No?
> 
> It is the errp that will be set if there is no "accel" is available that will fail the
> tests. Another way to avoid object_property_find() is to pass NULL for errp in
> object_property_get_bool(), but I think again when we introduce "accel" we have
> change that into either errp/error_abort here.
> 
> I am not sure there is any other way to handle this.

This path is to set MSI GPA, which could be optional, right? So,
we don't really need to exit when accel is not found or accel is
set to "n".

In that case, could we just use NULL even when "accel" is added?
Why do we use error_abort or errp?

Nicolin

> -----Original Message-----
> From: Nicolin Chen <nicolinc@nvidia.com>
> Sent: 24 November 2025 18:34
> To: Shameer Kolothum <skolothumtho@nvidia.com>
> Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org;
> eric.auger@redhat.com; peter.maydell@linaro.org; Jason Gunthorpe
> <jgg@nvidia.com>; ddutile@redhat.com; berrange@redhat.com; Nathan
> Chen <nathanc@nvidia.com>; Matt Ochs <mochs@nvidia.com>;
> smostafa@google.com; wangzhou1@hisilicon.com;
> jiangkunkun@huawei.com; jonathan.cameron@huawei.com;
> zhangfei.gao@linaro.org; zhenzhong.duan@intel.com; yi.l.liu@intel.com;
> Krishnakant Jaju <kjaju@nvidia.com>
> Subject: Re: [PATCH v6 17/33] hw/arm/smmuv3: Add support for providing a
> direct MSI doorbell GPA
> 
> On Mon, Nov 24, 2025 at 12:05:38AM -0800, Shameer Kolothum wrote:
> > > > > > +        if (object_property_find(OBJECT(dev), "accel") &&
> > > > > > +            object_property_get_bool(OBJECT(dev), "accel",
> &error_abort)) {
> > > > >
> > > > > Do we need object_property_find()? A later patch seems to drop it.
> > > > > Perhaps we shouldn't add it in the first place?
> > > >
> > > > We need that at this stage as we haven't added the "accel" property yet
> > > > and that will cause "make check" tests to fail without that.
> > > >
> > > > We remove it once we introduce "accel" property later.
> > >
> > > Hmm, I assume object_property_get_bool() would return false when
> > > "accel" is not available yet? No?
> >
> > It is the errp that will be set if there is no "accel" is available that will fail the
> > tests. Another way to avoid object_property_find() is to pass NULL for errp
> in
> > object_property_get_bool(), but I think again when we introduce "accel" we
> have
> > change that into either errp/error_abort here.
> >
> > I am not sure there is any other way to handle this.
> 
> This path is to set MSI GPA, which could be optional, right? So,
> we don't really need to exit when accel is not found or accel is
> set to "n".
> 
> In that case, could we just use NULL even when "accel" is added?
> Why do we use error_abort or errp?

I cant recollect why I used error_abort here in the first place. May be
since we used that for the "primary-bus" above. But as you said,
we could argue "accel" is optional property. However, if we pass NULL
and then for some other reason it fails(even if accel=on),
we will end up ignoring the error completely and silently boot without
"accel" ?

The  object_property_get_bool(..., &error_abort) is used in
virt-acpi-build.c as well.

Still not sure "NULL" is a good idea or not.

Thanks,
Shameer

On Mon, Nov 24, 2025 at 11:01:57AM -0800, Shameer Kolothum wrote:
> > On Mon, Nov 24, 2025 at 12:05:38AM -0800, Shameer Kolothum wrote:
> > > > > > > +        if (object_property_find(OBJECT(dev), "accel") &&
> > > > > > > +            object_property_get_bool(OBJECT(dev), "accel",
> > &error_abort)) {
> > > > > >
> > > > > > Do we need object_property_find()? A later patch seems to drop it.
> > > > > > Perhaps we shouldn't add it in the first place?
> > > > >
> > > > > We need that at this stage as we haven't added the "accel" property yet
> > > > > and that will cause "make check" tests to fail without that.
> > > > >
> > > > > We remove it once we introduce "accel" property later.
> > > >
> > > > Hmm, I assume object_property_get_bool() would return false when
> > > > "accel" is not available yet? No?
> > >
> > > It is the errp that will be set if there is no "accel" is available that will fail the
> > > tests. Another way to avoid object_property_find() is to pass NULL for errp
> > in
> > > object_property_get_bool(), but I think again when we introduce "accel" we
> > have
> > > change that into either errp/error_abort here.
> > >
> > > I am not sure there is any other way to handle this.
> > 
> > This path is to set MSI GPA, which could be optional, right? So,
> > we don't really need to exit when accel is not found or accel is
> > set to "n".
> > 
> > In that case, could we just use NULL even when "accel" is added?
> > Why do we use error_abort or errp?
> 
> I cant recollect why I used error_abort here in the first place. May be
> since we used that for the "primary-bus" above. But as you said,
> we could argue "accel" is optional property. However, if we pass NULL
> and then for some other reason it fails(even if accel=on),
> we will end up ignoring the error completely and silently boot without
> "accel" ?

OK. I see now..

What we want are:
1) When "accel" isn't defined, don't abort it by calling that
   object_property_find() to make msi-gpa optional.
2) When "accel" is defined, remove the object_property_find()
   because object_property_get_bool() will never abort. And it
   will make the msi-gpa optional depending on accel=on/off.

Perhaps we should re-organize the patch sequence, putting the
MSI one later. Checking the "accel" before we introduce it is
a bit odd. But any way, I think we are fine here, if you don't
want to change.

Thanks
Nicolin