[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH 1/2] x86/shadow: sanitize iommu_snoop usage


  • To: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Fri, 13 Jan 2023 09:47:40 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=PIN7bYME3B1gixZ9fvJuwYJvOqsQq3RxKGZEx+HCKzM=; b=CCxguQIoxLhGj1nARBQ3OCJP9W45jvd85jKIXF7a2E6VS4X4lCQ0XYef5nieJIz1osyMAWuZJsulp3drkrt+4LPjlRCesLTlgf9rdbCdx8mDf8HcaWhj3tHy8co6UiBin3eg0ZFEe1SklInxp1TU1+B0sd7dFKFW6lMIveKs7k6MfTpan7laquOBC/AG0jlnSXLRvAXYrpCXLUreXYHFAqcgddHOVfEz6XHrtRG2NxNYQbDO2w9gWDtF/TiozCswnK2NKUDftiti50iKehtCfeILBd0zYYiSXOeL4zHcJTYJT1M2zI4UGrjM17xguo2CTQ9LugHUAUfkG+dvP03/RQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=iRmT0ij8gAtOOt4rCEN/vyCQ7Aso5MV0XWank3L9GX8Pm9kdJRjqWx/qqSwvB8d/eNZnZDKpwQFamjhtOleQnz5PVlKfPIbNUt21UtVhKO2ThDlXjEK14uZfOXkOfizcNQ0ZyVj3GUtEq40jNJTdH3FA5Rc3kV0QBoSYM15K72gT81P1NIdMMGRyeysQj99x6ND9ykfmv7FZMsja/efWtUwRHwodik/2u734w1ZHBoFS6ll2lJgdyMac1t0cRSZFtUlmDOn+6/aEOTMvvpZbG55dfOt3Uyc3ahHsVSCzXBYpBl6/CVAsTW2Cm/Mdab15awaPbnJasv9VsEdSJBeSlQ==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, Tim Deegan <tim@xxxxxxx>, Xenia Ragiadakou <burzalodowa@xxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>
  • Delivery-date: Fri, 13 Jan 2023 08:47:48 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

First of all the variable is meaningful only when an IOMMU is in use for
a guest. Qualify the check accordingly, like done elsewhere. Furthermore
the controlling command line option is supposed to take effect on VT-d
only. Since command line parsing happens before we know whether we're
going to use VT-d, force the variable back to set when instead running
with AMD IOMMU(s).

Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
---
I was first considering to add the extra check to the outermost
enclosing if(), but I guess that would break the (questionable) case of
assigning MMIO ranges directly by address. The way it's done now also
better fits the existing checks, in particular the ones in p2m-ept.c.

Note that the #ifndef is put there in anticipation of iommu_snoop
becoming a #define when !IOMMU_INTEL (see
https://lists.xen.org/archives/html/xen-devel/2023-01/msg00103.html
and replies).

In _sh_propagate() I'm further puzzled: The iomem_access_permitted()
certainly suggests very bad things could happen if it returned false
(i.e. in the implicit "else" case). The assumption looks to be that no
bad "target_mfn" can make it there. But overall things might end up
looking more sane (and being cheaper) when simply using "mmio_mfn"
instead.

--- a/xen/arch/x86/mm/shadow/multi.c
+++ b/xen/arch/x86/mm/shadow/multi.c
@@ -571,7 +571,7 @@ _sh_propagate(struct vcpu *v,
                             gfn_to_paddr(target_gfn),
                             mfn_to_maddr(target_mfn),
                             X86_MT_UC);
-                else if ( iommu_snoop )
+                else if ( is_iommu_enabled(d) && iommu_snoop )
                     sflags |= pat_type_2_pte_flags(X86_MT_WB);
                 else
                     sflags |= get_pat_flags(v,
--- a/xen/drivers/passthrough/x86/iommu.c
+++ b/xen/drivers/passthrough/x86/iommu.c
@@ -56,6 +56,13 @@ void __init acpi_iommu_init(void)
     if ( !acpi_disabled )
     {
         ret = acpi_dmar_init();
+
+#ifndef iommu_snoop
+        /* A command line override for snoop control affects VT-d only. */
+        if ( ret )
+            iommu_snoop = true;
+#endif
+
         if ( ret == -ENODEV )
             ret = acpi_ivrs_init();
     }




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.