Hi Xingang,
On 5/25/21 5:50 AM, Wang Xingang wrote:
> From: Xingang Wang <wangxinga...@huawei.com>
>
> This add a bypass_iommu option for arm virt machine,
> the option can be used in this manner:
> qemu -machine virt,iommu=smmuv3,bypass_iommu=true
This still looks confusing to me. On one hand we say that for the virt
machine the iommu is set to smmuv3 and we say bypass_iommu=true on the
virt machine option line
It is not straightforward that the bypass_iommu only relates to devices
plugged onto the "default" root bus.
At least the name of the property should reflect that I think
> Signed-off-by: Xingang Wang <wangxinga...@huawei.com>
> ---
> hw/arm/virt.c | 26 ++++++++++++++++++++++++++
> include/hw/arm/virt.h | 1 +
> 2 files changed, 27 insertions(+)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 840758666d..49d8a801ed 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1364,6 +1364,7 @@ static void create_pcie(VirtMachineState *vms)
> }
>
> pci = PCI_HOST_BRIDGE(dev);
> + pci->bypass_iommu = vms->bypass_iommu;
> vms->bus = pci->bus;
> if (vms->bus) {
> for (i = 0; i < nb_nics; i++) {
> @@ -2319,6 +2320,21 @@ static void virt_set_iommu(Object *obj, const char
> *value, Error **errp)
> }
> }
>
> +static bool virt_get_bypass_iommu(Object *obj, Error **errp)
> +{
> + VirtMachineState *vms = VIRT_MACHINE(obj);
> +
> + return vms->bypass_iommu;
> +}
> +
> +static void virt_set_bypass_iommu(Object *obj, bool value,
> + Error **errp)
> +{
> + VirtMachineState *vms = VIRT_MACHINE(obj);
> +
> + vms->bypass_iommu = value;
> +}
> +
> static CpuInstanceProperties
> virt_cpu_index_to_props(MachineState *ms, unsigned cpu_index)
> {
> @@ -2656,6 +2672,13 @@ static void virt_machine_class_init(ObjectClass *oc,
> void *data)
> "Set the IOMMU type. "
> "Valid values are none and
> smmuv3");
>
> + object_class_property_add_bool(oc, "bypass_iommu",
> + virt_get_bypass_iommu,
> + virt_set_bypass_iommu);
> + object_class_property_set_description(oc, "bypass_iommu",
> + "Set on/off to enable/disable "
> + "bypass_iommu for primary bus");
> +
> object_class_property_add_bool(oc, "ras", virt_get_ras,
> virt_set_ras);
> object_class_property_set_description(oc, "ras",
> @@ -2723,6 +2746,9 @@ static void virt_instance_init(Object *obj)
> /* Default disallows iommu instantiation */
> vms->iommu = VIRT_IOMMU_NONE;
>
> + /* The primary bus is attached to iommu by default */
> + vms->bypass_iommu = false;
I don't fully master the PCI topology but I think you should clarify
which primary bus we talk about. AFAIU PXB's bus also is a primary bus.
Thanks
Eric
> +
> /* Default disallows RAS instantiation */
> vms->ras = false;
>
> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> index 921416f918..82bceadb82 100644
> --- a/include/hw/arm/virt.h
> +++ b/include/hw/arm/virt.h
> @@ -147,6 +147,7 @@ struct VirtMachineState {
> OnOffAuto acpi;
> VirtGICType gic_version;
> VirtIOMMUType iommu;
> + bool bypass_iommu;
> VirtMSIControllerType msi_controller;
> uint16_t virtio_iommu_bdf;
> struct arm_boot_info bootinfo;
