From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47206) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZkFeA-0006cp-4r for qemu-devel@nongnu.org; Thu, 08 Oct 2015 14:10:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZkFe6-0001hz-4K for qemu-devel@nongnu.org; Thu, 08 Oct 2015 14:10:30 -0400 Received: from mail-wi0-x231.google.com ([2a00:1450:400c:c05::231]:35768) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZkFe5-0001hl-RN for qemu-devel@nongnu.org; Thu, 08 Oct 2015 14:10:26 -0400 Received: by wicge5 with SMTP id ge5so37228555wic.0 for ; Thu, 08 Oct 2015 11:10:25 -0700 (PDT) References: <1444359237-27224-1-git-send-email-davidkiarie4@gmail.com> <1444359237-27224-2-git-send-email-davidkiarie4@gmail.com> From: Marcel Apfelbaum Message-ID: <5616B18E.5080100@gmail.com> Date: Thu, 8 Oct 2015 21:10:22 +0300 MIME-Version: 1.0 In-Reply-To: <1444359237-27224-2-git-send-email-davidkiarie4@gmail.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/4] hw/core: Add iommu to machine properties Reply-To: marcel@redhat.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Kiarie , qemu-devel@nongnu.org, jan.kiszka@web.de, valentine.sinitsyn@gmail.com, mst@redhat.com On 10/09/2015 05:53 AM, David Kiarie wrote: > From: David > > Add iommu to machine properties in preparation of introducing > AMD IOMMU > > Signed-off-by: David Kiarie > --- > hw/core/machine.c | 25 +++++++++++++++++++++++++ > include/hw/boards.h | 2 ++ > 2 files changed, 27 insertions(+) > > diff --git a/hw/core/machine.c b/hw/core/machine.c > index 51ed6b2..8cc7461 100644 > --- a/hw/core/machine.c > +++ b/hw/core/machine.c > @@ -269,6 +269,20 @@ static void machine_set_iommu(Object *obj, bool value, Error **errp) > ms->iommu = value; > } > > +static bool machine_get_amd_iommu(Object *obj, Error **errp) > +{ > + MachineState *ms = MACHINE(obj); > + > + return ms->amd_iommu; > +} > + > +static void machine_set_amd_iommu(Object *obj, bool value, Error **errp) > +{ > + MachineState *ms = MACHINE(obj); > + > + ms->amd_iommu = value; > +} > + > static void machine_set_suppress_vmdesc(Object *obj, bool value, Error **errp) > { > MachineState *ms = MACHINE(obj); > @@ -420,6 +434,12 @@ static void machine_initfn(Object *obj) > object_property_set_description(obj, "iommu", > "Set on/off to enable/disable Intel IOMMU (VT-d)", > NULL); > + object_property_add_bool(obj, "amd-iommu", > + machine_get_amd_iommu, > + machine_set_amd_iommu, NULL); > + object_property_set_description(obj, "amd-iommu", > + "Set on/off to enable/disable AMD-Vi", > + NULL); > object_property_add_bool(obj, "suppress-vmdesc", > machine_get_suppress_vmdesc, > machine_set_suppress_vmdesc, NULL); > @@ -456,6 +476,11 @@ bool machine_iommu(MachineState *machine) > return machine->iommu; > } > > +bool machine_amd_iommu(MachineState *machine) > +{ > + return machine->amd_iommu; > +} > + > bool machine_kernel_irqchip_allowed(MachineState *machine) > { > return machine->kernel_irqchip_allowed; > diff --git a/include/hw/boards.h b/include/hw/boards.h > index 566a5ca..c8424f7 100644 > --- a/include/hw/boards.h > +++ b/include/hw/boards.h > @@ -54,6 +54,7 @@ extern MachineState *current_machine; > > bool machine_usb(MachineState *machine); > bool machine_iommu(MachineState *machine); > +bool machine_amd_iommu(MachineState *machine); > bool machine_kernel_irqchip_allowed(MachineState *machine); > bool machine_kernel_irqchip_required(MachineState *machine); > int machine_kvm_shadow_mem(MachineState *machine); > @@ -140,6 +141,7 @@ struct MachineState { > bool igd_gfx_passthru; > char *firmware; > bool iommu; > + bool amd_iommu; > bool suppress_vmdesc; > > ram_addr_t ram_size; > Hi, I think we should add the property to PC_MACHINE class because it make sense only for PC and Q35 machines (right?). MACHINE is the base class for all the architectures. On the same note, "iommu" refers to intel_iommu and maybe we should move it also to the PC_MACHINE. I understand that it is not in the scope of your series, we can add a patch later. Another possibility would be to keep the same "iommu" property, but change it from boolean to off|intel|amd|... . This will break backward compatibility, on the other hand having iommu referring to Intel only when we have multiple iommu implementations is ugly IMHO. Thanks, Marcel