Hi Shameer,
On 3/11/25 3:10 PM, Shameer Kolothum wrote:
> Allow cold-plug smmuv3-accel to virt If the machine wide smmuv3
> is not specified.
>
> No FDT support is added for now.
>
> Signed-off-by: Shameer Kolothum <shameerali.kolothum.th...@huawei.com>
> ---
> hw/arm/virt.c | 12 ++++++++++++
> hw/core/sysbus-fdt.c | 1 +
> include/hw/arm/virt.h | 1 +
> 3 files changed, 14 insertions(+)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 4a5a9666e9..84a323da55 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -73,6 +73,7 @@
> #include "qobject/qlist.h"
> #include "standard-headers/linux/input.h"
> #include "hw/arm/smmuv3.h"
> +#include "hw/arm/smmuv3-accel.h"
> #include "hw/acpi/acpi.h"
> #include "target/arm/cpu-qom.h"
> #include "target/arm/internals.h"
> @@ -2911,6 +2912,16 @@ static void virt_machine_device_plug_cb(HotplugHandler
> *hotplug_dev,
>
> platform_bus_link_device(PLATFORM_BUS_DEVICE(vms->platform_bus_dev),
> SYS_BUS_DEVICE(dev));
> }
> + if (object_dynamic_cast(OBJECT(dev), TYPE_ARM_SMMUV3_ACCEL)) {
> + if (vms->iommu == VIRT_IOMMU_SMMUV3) {
maybe just check whether it is != VIRT_IOMMU_NONE?
> + error_setg(errp,
> + "iommu=smmuv3 is already specified. can't create
> smmuv3-accel dev");
I would clearly state "iommu=smmuv3 virt machine option is alreadt set"
and use an error hint to say both are not compatible.
> + return;
> + }
> + if (vms->iommu != VIRT_IOMMU_SMMUV3_ACCEL) {
> + vms->iommu = VIRT_IOMMU_SMMUV3_ACCEL;
I know there were quite a lot of dicussions on the 1st multi
instantiation series related to the way we instanatiate that device and
maybe I missed some blockers but why wouldn't we allow the instantiation
of the legacy smmu device with -device too. I think this would be
simpler for libvirt and we would somehow deprecate the machine option
method? would that make a problem if you were to use -device smmu,accel
or something alike?
> + }
> + }
> }
>
> if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
> @@ -3120,6 +3131,7 @@ static void virt_machine_class_init(ObjectClass *oc,
> void *data)
> machine_class_allow_dynamic_sysbus_dev(mc, TYPE_VFIO_AMD_XGBE);
> machine_class_allow_dynamic_sysbus_dev(mc, TYPE_RAMFB_DEVICE);
> machine_class_allow_dynamic_sysbus_dev(mc, TYPE_VFIO_PLATFORM);
> + machine_class_allow_dynamic_sysbus_dev(mc, TYPE_ARM_SMMUV3_ACCEL);
> #ifdef CONFIG_TPM
> machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_TIS_SYSBUS);
> #endif
> diff --git a/hw/core/sysbus-fdt.c b/hw/core/sysbus-fdt.c
> index 774c0aed41..c8502ad830 100644
> --- a/hw/core/sysbus-fdt.c
> +++ b/hw/core/sysbus-fdt.c
> @@ -489,6 +489,7 @@ static const BindingEntry bindings[] = {
> #ifdef CONFIG_LINUX
> TYPE_BINDING(TYPE_VFIO_CALXEDA_XGMAC, add_calxeda_midway_xgmac_fdt_node),
> TYPE_BINDING(TYPE_VFIO_AMD_XGBE, add_amd_xgbe_fdt_node),
> + TYPE_BINDING("arm-smmuv3-accel", no_fdt_node),
use the define instead.
to me this patch should be moved at the end of the series when the
device is fully functional.
> VFIO_PLATFORM_BINDING("amd,xgbe-seattle-v1a", add_amd_xgbe_fdt_node),
> #endif
> #ifdef CONFIG_TPM
> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> index c8e94e6aed..849d1cd5b5 100644
> --- a/include/hw/arm/virt.h
> +++ b/include/hw/arm/virt.h
> @@ -92,6 +92,7 @@ enum {
> typedef enum VirtIOMMUType {
> VIRT_IOMMU_NONE,
> VIRT_IOMMU_SMMUV3,
> + VIRT_IOMMU_SMMUV3_ACCEL,
> VIRT_IOMMU_VIRTIO,
> } VirtIOMMUType;
>
Thanks
Eric
