[PATCH v2 0/7] x86/pvh: fix fixes for PVH Dom0

[PATCH v2 0/7] x86/pvh: fix fixes for PVH Dom0

[Roger Pau Monne]

Hello,

The following series contain miscellaneous fixes for a PVH Dom0. I've
found out this issues while trying to boot on an AMD EPYC box.

The series can be found on my git repo:

git://xenbits.xen.org/people/royger/xen.git fixes-pvh-v2

Thanks, Roger.

Roger Pau Monne (7):
  x86/pvh: fix TSC mode setup for PVH Dom0
  x86/hvm: introduce a define for the debug output IO port
  x86/pvh: allow PVH Dom0 to use the debug IO port console
  vpci: fix updating the command register
  vpci: fix execution of long running operations
  vpci/msix: carve p2m hole for MSIX MMIO regions
  amd/pvh: enable ACPI C1E disable quirk on PVH Dom0

 docs/misc/xen-command-line.markdown | 14 ++++-
 xen/arch/x86/cpu/amd.c              | 13 +++--
 xen/arch/x86/dom0_build.c           | 20 +++++++
 xen/arch/x86/hvm/hvm.c              |  4 +-
 xen/arch/x86/hvm/ioreq.c            |  3 --
 xen/arch/x86/hvm/svm/svm.c          | 21 ++++++++
 xen/arch/x86/time.c                 |  2 +-
 xen/common/domain.c                 |  2 +
 xen/drivers/char/console.c          |  2 +-
 xen/drivers/vpci/header.c           | 84 ++++++++++++++++++-----------
 xen/drivers/vpci/msix.c             | 40 ++++++++++++++
 xen/include/asm-x86/amd.h           |  3 ++
 xen/include/public/arch-x86/xen.h   |  6 +++
 xen/include/xen/vpci.h              | 18 +++----
 14 files changed, 179 insertions(+), 53 deletions(-)

-- 
2.19.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v2 1/7] x86/pvh: fix TSC mode setup for PVH Dom0

[Roger Pau Monne]

A PVH Dom0 might use TSC scaling or other HVM specific TSC
adjustments, so only short-circuit the TSC setup for a classic PV
Dom0.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/x86/time.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
index 553698d4ab..03f792e7e5 100644
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -2125,7 +2125,7 @@ void tsc_set_info(struct domain *d,
 {
     ASSERT(!is_system_domain(d));
 
-    if ( is_hardware_domain(d) )
+    if ( is_pv_domain(d) && is_hardware_domain(d) )
     {
         d->arch.vtsc = 0;
         return;
-- 
2.19.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v2 2/7] x86/hvm: introduce a define for the debug output IO port

[Roger Pau Monne]

No functional change intended.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
---
 xen/arch/x86/hvm/hvm.c            | 4 ++--
 xen/drivers/char/console.c        | 2 +-
 xen/include/public/arch-x86/xen.h | 6 ++++++
 3 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index af13de3745..c3e6cccc4a 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -545,7 +545,7 @@ static int hvm_print_line(
     struct domain *cd = current->domain;
     char c = *val;
 
-    ASSERT(bytes == 1 && port == 0xe9);
+    ASSERT(bytes == 1 && port == XEN_HVM_DEBUGCONS_IOPORT);
 
     /* Deny any input requests. */
     if ( dir != IOREQ_WRITE )
@@ -654,7 +654,7 @@ int hvm_domain_initialise(struct domain *d)
 
     rtc_init(d);
 
-    register_portio_handler(d, 0xe9, 1, hvm_print_line);
+    register_portio_handler(d, XEN_HVM_DEBUGCONS_IOPORT, 1, hvm_print_line);
 
     if ( hvm_tsc_scaling_supported )
         d->arch.hvm.tsc_scaling_ratio = hvm_default_tsc_scaling_ratio;
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 3b75f7a472..734abd2552 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -458,7 +458,7 @@ static inline void xen_console_write_debug_port(const char *buf, size_t len)
     unsigned long tmp;
     asm volatile ( "rep outsb;"
                    : "=&S" (tmp), "=&c" (tmp)
-                   : "0" (buf), "1" (len), "d" (0xe9) );
+                   : "0" (buf), "1" (len), "d" (XEN_HVM_DEBUGCONS_IOPORT) );
 }
 #endif
 
diff --git a/xen/include/public/arch-x86/xen.h b/xen/include/public/arch-x86/xen.h
index 69ee4bc40d..5c8f7dd40f 100644
--- a/xen/include/public/arch-x86/xen.h
+++ b/xen/include/public/arch-x86/xen.h
@@ -346,6 +346,12 @@ struct xen_arch_domainconfig {
 #define XEN_CPUID          XEN_EMULATE_PREFIX "cpuid"
 #endif
 
+/*
+ * Debug console IO port, also called "port E9 hack". Each character written
+ * to this IO port will be printed on the hypervisor console.
+ */
+#define XEN_HVM_DEBUGCONS_IOPORT 0xe9
+
 #endif /* __XEN_PUBLIC_ARCH_X86_XEN_H__ */
 
 /*
-- 
2.19.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v2 3/7] x86/pvh: allow PVH Dom0 to use the debug IO port console

[Roger Pau Monne]

Add an option to allow trapping accesses to IO port 0xe9 for a PVH
Dom0, so it can print to the HVM debug console. Note this is not
enabled by default in order to prevent clashes with hardware on the
system using 0xe9.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
Changes since v1:
 - Use a define for 0xe9.
 - Expand 'List of' in the Xen command doc.
---
 docs/misc/xen-command-line.markdown | 14 +++++++++++++-
 xen/arch/x86/dom0_build.c           | 14 ++++++++++++++
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
index 2c7046eb86..2fc0448f07 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -664,7 +664,7 @@ trace feature is only enabled in debugging builds of Xen.
 Specify the bit width of the DMA heap.
 
 ### dom0 (x86)
-> `= List of [ pvh | shadow ]`
+> `= List of [ pvh | shadow | debug-ioport ]`
 
 > Sub-options:
 
@@ -681,6 +681,18 @@ Flag that makes a dom0 boot in PVHv2 mode.
 Flag that makes a dom0 use shadow paging. Only works when "pvh" is
 enabled.
 
+> `debug-ioport`
+
+> Default: `false`
+
+Flag that enables the HVM debug console for a PVH Dom0. Xen will trap accesses
+to IO port 0xe9 so that Dom0 kernel can print output using this IO port before
+setting up the hypercall page.
+
+Note this option is not enabled by default because it might clash with hardware
+on the system using IO port 0xe9 (XEN_HVM_DEBUGCONS_IOPORT) and should only be
+used for debug purposes.
+
 ### dom0-iommu
 > `= List of [ passthrough | strict | map-inclusive ]`
 
diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
index dcd7afb058..7b1aaaa7cb 100644
--- a/xen/arch/x86/dom0_build.c
+++ b/xen/arch/x86/dom0_build.c
@@ -212,12 +212,16 @@ struct vcpu *__init alloc_dom0_vcpu0(struct domain *dom0)
 bool __initdata opt_dom0_shadow;
 #endif
 bool __initdata dom0_pvh;
+#ifdef CONFIG_HVM
+bool __initdata opt_dom0_debug_ioport;
+#endif
 
 /*
  * List of parameters that affect Dom0 creation:
  *
  *  - pvh               Create a PVHv2 Dom0.
  *  - shadow            Use shadow paging for Dom0.
+ *  - debug-ioport      Trap accesses to 0xe9 (HVM debug console).
  */
 static int __init parse_dom0_param(const char *s)
 {
@@ -236,6 +240,10 @@ static int __init parse_dom0_param(const char *s)
 #ifdef CONFIG_SHADOW_PAGING
         else if ( (val = parse_boolean("shadow", s, ss)) >= 0 )
             opt_dom0_shadow = val;
+#endif
+#ifdef CONFIG_HVM
+        else if ( (val = parse_boolean("debug-ioport", s, ss)) >= 0 )
+            opt_dom0_debug_ioport = val;
 #endif
         else
             rc = -EINVAL;
@@ -433,6 +441,12 @@ int __init dom0_setup_permissions(struct domain *d)
         rc |= ioports_deny_access(d, pmtmr_ioport, pmtmr_ioport + 3);
     /* PCI configuration space (NB. 0xcf8 has special treatment). */
     rc |= ioports_deny_access(d, 0xcfc, 0xcff);
+#ifdef CONFIG_HVM
+    if ( is_hvm_domain(d) && opt_dom0_debug_ioport )
+        /* HVM debug console IO port. */
+        rc |= ioports_deny_access(d, XEN_HVM_DEBUGCONS_IOPORT,
+                                  XEN_HVM_DEBUGCONS_IOPORT);
+#endif
     /* Command-line I/O ranges. */
     process_dom0_ioports_disable(d);
 
-- 
2.19.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v2 4/7] vpci: fix updating the command register

[Roger Pau Monne]

When switching the memory decoding bit in the command register the
rest of the changes where dropped, leading to only the memory decoding
bit being updated.

Fix this by unconditionally writing the guest-requested command except
for the memory decoding bit, which will be updated once the p2m
changes are performed.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
---
 xen/drivers/vpci/header.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
index 0ec4c082a6..9234de9b26 100644
--- a/xen/drivers/vpci/header.c
+++ b/xen/drivers/vpci/header.c
@@ -333,8 +333,10 @@ static void cmd_write(const struct pci_dev *pdev, unsigned int reg,
          * hoping the guest will realize and try again.
          */
         modify_bars(pdev, cmd & PCI_COMMAND_MEMORY, false);
-    else
-        pci_conf_write16(pdev->seg, pdev->bus, slot, func, reg, cmd);
+
+    /* Write the new command without updating the memory decoding bit. */
+    cmd = (cmd & ~PCI_COMMAND_MEMORY) | (current_cmd & PCI_COMMAND_MEMORY);
+    pci_conf_write16(pdev->seg, pdev->bus, slot, func, reg, cmd);
 }
 
 static void bar_write(const struct pci_dev *pdev, unsigned int reg,
-- 
2.19.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v2 5/7] vpci: fix execution of long running operations

[Roger Pau Monne]

BAR map/unmap is a long running operation that needs to be preempted
in order to avoid overrunning the assigned vCPU time (or even
triggering the watchdog).

Current logic for this preemption is wrong, and won't work at all for
AMD since only Intel makes use of hvm_io_pending (and even in that
case the current code is wrong).

Instead move the code that performs the mapping/unmapping to
a tasklet and pause the guest vCPU until the {un}mapping is done and
the memory decoding bit has been toggled.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
[ioreq parts]
Reviewed-by: Paul Durrant <paul.durrant@citrix.com>
---
Cc: Paul Durrant <paul.durrant@citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
---
 xen/arch/x86/hvm/ioreq.c  |  3 --
 xen/common/domain.c       |  2 ++
 xen/drivers/vpci/header.c | 70 +++++++++++++++++++++++----------------
 xen/include/xen/vpci.h    | 15 +++------
 4 files changed, 48 insertions(+), 42 deletions(-)

diff --git a/xen/arch/x86/hvm/ioreq.c b/xen/arch/x86/hvm/ioreq.c
index e2e755a8a1..58a86f9225 100644
--- a/xen/arch/x86/hvm/ioreq.c
+++ b/xen/arch/x86/hvm/ioreq.c
@@ -85,9 +85,6 @@ bool hvm_io_pending(struct vcpu *v)
     struct hvm_ioreq_server *s;
     unsigned int id;
 
-    if ( has_vpci(d) && vpci_process_pending(v) )
-        return true;
-
     FOR_EACH_IOREQ_SERVER(d, id, s)
     {
         struct hvm_ioreq_vcpu *sv;
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 65151e2ac4..54d2c26bd9 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -153,6 +153,8 @@ struct vcpu *vcpu_create(
 
     grant_table_init_vcpu(v);
 
+    vpci_init_vcpu(v);
+
     if ( !zalloc_cpumask_var(&v->cpu_hard_affinity) ||
          !zalloc_cpumask_var(&v->cpu_hard_affinity_tmp) ||
          !zalloc_cpumask_var(&v->cpu_hard_affinity_saved) ||
diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
index 9234de9b26..4af85d3c02 100644
--- a/xen/drivers/vpci/header.c
+++ b/xen/drivers/vpci/header.c
@@ -118,39 +118,48 @@ static void modify_decoding(const struct pci_dev *pdev, bool map, bool rom_only)
                      cmd);
 }
 
-bool vpci_process_pending(struct vcpu *v)
+static void vpci_process_pending(unsigned long data)
 {
-    if ( v->vpci.mem )
+    struct vcpu *v = (void *)data;
+    struct map_data map_data = {
+        .d = v->domain,
+        .map = v->vpci.map,
+    };
+    int rc;
+
+    ASSERT(v->vpci.mem && atomic_read(&v->pause_count));
+
+    while ( (rc = rangeset_consume_ranges(v->vpci.mem, map_range, &map_data)) ==
+            -ERESTART )
     {
-        struct map_data data = {
-            .d = v->domain,
-            .map = v->vpci.map,
-        };
-        int rc = rangeset_consume_ranges(v->vpci.mem, map_range, &data);
-
-        if ( rc == -ERESTART )
-            return true;
-
-        spin_lock(&v->vpci.pdev->vpci->lock);
-        /* Disable memory decoding unconditionally on failure. */
-        modify_decoding(v->vpci.pdev, !rc && v->vpci.map,
-                        !rc && v->vpci.rom_only);
-        spin_unlock(&v->vpci.pdev->vpci->lock);
-
-        rangeset_destroy(v->vpci.mem);
-        v->vpci.mem = NULL;
-        if ( rc )
-            /*
-             * FIXME: in case of failure remove the device from the domain.
-             * Note that there might still be leftover mappings. While this is
-             * safe for Dom0, for DomUs the domain will likely need to be
-             * killed in order to avoid leaking stale p2m mappings on
-             * failure.
-             */
-            vpci_remove_device(v->vpci.pdev);
+        tasklet_schedule(&v->vpci.task);
+        return;
     }
 
-    return false;
+    spin_lock(&v->vpci.pdev->vpci->lock);
+    /* Disable memory decoding unconditionally on failure. */
+    modify_decoding(v->vpci.pdev, !rc && v->vpci.map,
+                    !rc && v->vpci.rom_only);
+    spin_unlock(&v->vpci.pdev->vpci->lock);
+
+    rangeset_destroy(v->vpci.mem);
+    v->vpci.mem = NULL;
+    if ( rc )
+        /*
+         * FIXME: in case of failure remove the device from the domain.
+         * Note that there might still be leftover mappings. While this is
+         * safe for Dom0, for DomUs the domain will likely need to be
+         * killed in order to avoid leaking stale p2m mappings on
+         * failure.
+         */
+        vpci_remove_device(v->vpci.pdev);
+
+    vcpu_unpause(v);
+}
+
+void vpci_init_vcpu(struct vcpu *v)
+{
+    tasklet_init(&v->vpci.task, vpci_process_pending, (unsigned long)v);
 }
 
 static int __init apply_map(struct domain *d, const struct pci_dev *pdev,
@@ -183,6 +192,9 @@ static void defer_map(struct domain *d, struct pci_dev *pdev,
     curr->vpci.mem = mem;
     curr->vpci.map = map;
     curr->vpci.rom_only = rom_only;
+    /* Pause the vCPU and schedule the tasklet that will perform the mapping. */
+    vcpu_pause_nosync(curr);
+    tasklet_schedule(&curr->vpci.task);
 }
 
 static int modify_bars(const struct pci_dev *pdev, bool map, bool rom_only)
diff --git a/xen/include/xen/vpci.h b/xen/include/xen/vpci.h
index af2b8580ee..f97c48a8f1 100644
--- a/xen/include/xen/vpci.h
+++ b/xen/include/xen/vpci.h
@@ -49,11 +49,8 @@ uint32_t vpci_hw_read16(const struct pci_dev *pdev, unsigned int reg,
 uint32_t vpci_hw_read32(const struct pci_dev *pdev, unsigned int reg,
                         void *data);
 
-/*
- * Check for pending vPCI operations on this vcpu. Returns true if the vcpu
- * should not run.
- */
-bool __must_check vpci_process_pending(struct vcpu *v);
+/* Init per-vCPU vPCI data. */
+void vpci_init_vcpu(struct vcpu *v);
 
 struct vpci {
     /* List of vPCI handlers for a device. */
@@ -142,6 +139,8 @@ struct vpci {
 };
 
 struct vpci_vcpu {
+    /* Per-vCPU tasklet to enqueue pending operations. */
+    struct tasklet task;
     /* Per-vcpu structure to store state while {un}mapping of PCI BARs. */
     struct rangeset *mem;
     struct pci_dev *pdev;
@@ -233,11 +232,7 @@ static inline void vpci_write(pci_sbdf_t sbdf, unsigned int reg,
     ASSERT_UNREACHABLE();
 }
 
-static inline bool vpci_process_pending(struct vcpu *v)
-{
-    ASSERT_UNREACHABLE();
-    return false;
-}
+static inline void vpci_init_vcpu(struct vcpu *v) { }
 #endif
 
 #endif
-- 
2.19.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v2 6/7] vpci/msix: carve p2m hole for MSIX MMIO regions

[Roger Pau Monne]

Make sure the MSIX MMIO regions don't have p2m entries setup, so that
accesses to them trap into the hypervisor and can be handled by vpci.

This is a side-effect of commit 042678762 for PVH Dom0, which added
mappings for all the reserved regions into the Dom0 p2m.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>
---
 xen/drivers/vpci/header.c |  8 ++++++++
 xen/drivers/vpci/msix.c   | 40 +++++++++++++++++++++++++++++++++++++++
 xen/include/xen/vpci.h    |  3 +++
 3 files changed, 51 insertions(+)

diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
index 4af85d3c02..455dd4fc90 100644
--- a/xen/drivers/vpci/header.c
+++ b/xen/drivers/vpci/header.c
@@ -88,6 +88,14 @@ static void modify_decoding(const struct pci_dev *pdev, bool map, bool rom_only)
     uint16_t cmd;
     unsigned int i;
 
+    /*
+     * Make sure there are no mappings in the MSIX MMIO areas, so that accesses
+     * can be trapped (and emulated) by Xen when the memory decoding bit is
+     * enabled.
+     */
+    if ( map && !rom_only && vpci_make_msix_hole(pdev) )
+        return;
+
     for ( i = 0; i < ARRAY_SIZE(header->bars); i++ )
     {
         if ( !MAPPABLE_BAR(&header->bars[i]) )
diff --git a/xen/drivers/vpci/msix.c b/xen/drivers/vpci/msix.c
index 1960dae123..5759551724 100644
--- a/xen/drivers/vpci/msix.c
+++ b/xen/drivers/vpci/msix.c
@@ -21,6 +21,7 @@
 #include <xen/vpci.h>
 
 #include <asm/msi.h>
+#include <asm/p2m.h>
 
 #define VMSIX_SIZE(num) offsetof(struct vpci_msix, entries[num])
 
@@ -395,6 +396,45 @@ static const struct hvm_mmio_ops vpci_msix_table_ops = {
     .write = msix_write,
 };
 
+int vpci_make_msix_hole(const struct pci_dev *pdev)
+{
+    struct domain *d = pdev->domain;
+    unsigned int i;
+
+    if ( !pdev->vpci->msix )
+        return 0;
+
+    /* Make sure there's a hole for the MSIX table/PBA in the p2m. */
+    for ( i = 0; i < ARRAY_SIZE(pdev->vpci->msix->tables); i++ )
+    {
+        unsigned long start = PFN_DOWN(vmsix_table_addr(pdev->vpci, i));
+        unsigned long end = PFN_DOWN(vmsix_table_addr(pdev->vpci, i) +
+                                     vmsix_table_size(pdev->vpci, i) - 1);
+
+        for ( ; start <= end; start++ )
+        {
+            p2m_type_t t;
+            mfn_t mfn = get_gfn_query(d, start, &t);
+
+            if ( t == p2m_mmio_direct && mfn_x(mfn) == start )
+                    clear_identity_p2m_entry(d, start);
+            else if ( t != p2m_mmio_dm )
+            {
+                put_gfn(d, start);
+                gprintk(XENLOG_WARNING,
+                        "%04x:%02x:%02x.%u: existing mapping (mfn: %" PRI_mfn
+                        "type: %d) at %#lx clobbers MSIX MMIO area\n",
+                        pdev->seg, pdev->bus, PCI_SLOT(pdev->devfn),
+                        PCI_FUNC(pdev->devfn), mfn_x(mfn), t, start);
+                return -EEXIST;
+            }
+            put_gfn(d, start);
+        }
+    }
+
+    return 0;
+}
+
 static int init_msix(struct pci_dev *pdev)
 {
     struct domain *d = pdev->domain;
diff --git a/xen/include/xen/vpci.h b/xen/include/xen/vpci.h
index f97c48a8f1..e0c22ad81c 100644
--- a/xen/include/xen/vpci.h
+++ b/xen/include/xen/vpci.h
@@ -151,6 +151,9 @@ struct vpci_vcpu {
 #ifdef __XEN__
 void vpci_dump_msi(void);
 
+/* Make sure there's a hole in the p2m for the MSIX mmio areas. */
+int vpci_make_msix_hole(const struct pci_dev *pdev);
+
 /* Arch-specific vPCI MSI helpers. */
 void vpci_msi_arch_mask(struct vpci_msi *msi, const struct pci_dev *pdev,
                         unsigned int entry, bool mask);
-- 
2.19.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH v2 7/7] amd/pvh: enable ACPI C1E disable quirk on PVH Dom0

[Roger Pau Monne]

PV Dom0 has a quirk for some AMD processors, where enabling ACPI can
also enable C1E mode. Apply the same workaround as done on PV for a
PVH Dom0, which consist on trapping accesses to the SMI command IO
port and disabling C1E if ACPI is enabled.

Reported-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Cc: Brian Woods <brian.woods@amd.com>
---
 xen/arch/x86/cpu/amd.c     | 13 +++++++++----
 xen/arch/x86/dom0_build.c  | 14 ++++++++++----
 xen/arch/x86/hvm/svm/svm.c | 21 +++++++++++++++++++++
 xen/include/asm-x86/amd.h  |  3 +++
 4 files changed, 43 insertions(+), 8 deletions(-)

diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c
index c394c1c2ec..18a9e92b3c 100644
--- a/xen/arch/x86/cpu/amd.c
+++ b/xen/arch/x86/cpu/amd.c
@@ -44,6 +44,9 @@ integer_param("cpuid_mask_thermal_ecx", opt_cpuid_mask_thermal_ecx);
 s8 __read_mostly opt_allow_unsafe;
 boolean_param("allow_unsafe", opt_allow_unsafe);
 
+/* Signal whether the ACPI C1E quirk is required. */
+bool amd_acpi_c1e_quirk;
+
 static inline int rdmsr_amd_safe(unsigned int msr, unsigned int *lo,
 				 unsigned int *hi)
 {
@@ -427,7 +430,7 @@ static void disable_c1_ramping(void)
 	}
 }
 
-static void disable_c1e(void *unused)
+void amd_disable_c1e(void *unused)
 {
 	uint64_t msr_content;
 
@@ -447,7 +450,7 @@ static void check_disable_c1e(unsigned int port, u8 value)
 {
 	/* C1E is sometimes enabled during entry to ACPI mode. */
 	if ((port == acpi_smi_cmd) && (value == acpi_enable_value))
-		on_each_cpu(disable_c1e, NULL, 1);
+		on_each_cpu(amd_disable_c1e, NULL, 1);
 }
 
 /*
@@ -626,9 +629,11 @@ static void init_amd(struct cpuinfo_x86 *c)
 	switch(c->x86)
 	{
 	case 0xf ... 0x17:
-		disable_c1e(NULL);
-		if (acpi_smi_cmd && (acpi_enable_value | acpi_disable_value))
+		amd_disable_c1e(NULL);
+		if (acpi_smi_cmd && (acpi_enable_value | acpi_disable_value)) {
 			pv_post_outb_hook = check_disable_c1e;
+			amd_acpi_c1e_quirk = true;
+		}
 		break;
 	}
 
diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
index 7b1aaaa7cb..eca0255a43 100644
--- a/xen/arch/x86/dom0_build.c
+++ b/xen/arch/x86/dom0_build.c
@@ -12,6 +12,7 @@
 #include <xen/sched-if.h>
 #include <xen/softirq.h>
 
+#include <asm/amd.h>
 #include <asm/dom0_build.h>
 #include <asm/guest.h>
 #include <asm/hpet.h>
@@ -442,10 +443,15 @@ int __init dom0_setup_permissions(struct domain *d)
     /* PCI configuration space (NB. 0xcf8 has special treatment). */
     rc |= ioports_deny_access(d, 0xcfc, 0xcff);
 #ifdef CONFIG_HVM
-    if ( is_hvm_domain(d) && opt_dom0_debug_ioport )
-        /* HVM debug console IO port. */
-        rc |= ioports_deny_access(d, XEN_HVM_DEBUGCONS_IOPORT,
-                                  XEN_HVM_DEBUGCONS_IOPORT);
+    if ( is_hvm_domain(d) )
+    {
+        if ( opt_dom0_debug_ioport )
+            /* HVM debug console IO port. */
+            rc |= ioports_deny_access(d, XEN_HVM_DEBUGCONS_IOPORT,
+                                      XEN_HVM_DEBUGCONS_IOPORT);
+        if ( amd_acpi_c1e_quirk )
+            rc |= ioports_deny_access(d, acpi_smi_cmd, acpi_smi_cmd);
+    }
 #endif
     /* Command-line I/O ranges. */
     process_dom0_ioports_disable(d);
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index dd0aca4f53..2ed24a624c 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1272,6 +1272,24 @@ void svm_host_osvw_init()
     spin_unlock(&osvw_lock);
 }
 
+static int acpi_c1e_quirk(int dir, unsigned int port, unsigned int bytes,
+                          uint32_t *val)
+{
+    ASSERT(bytes == 1 && port == acpi_smi_cmd);
+
+    if ( dir == IOREQ_READ )
+    {
+        *val = inb(port);
+        return X86EMUL_OKAY;
+    }
+
+    outb(*val, port);
+    if ( *val == acpi_enable_value )
+       on_each_cpu(amd_disable_c1e, NULL, 1);
+
+    return X86EMUL_OKAY;
+}
+
 static int svm_domain_initialise(struct domain *d)
 {
     static const struct arch_csw csw = {
@@ -1284,6 +1302,9 @@ static int svm_domain_initialise(struct domain *d)
 
     svm_guest_osvw_init(d);
 
+    if ( amd_acpi_c1e_quirk )
+        register_portio_handler(d, acpi_smi_cmd, 1, acpi_c1e_quirk);
+
     return 0;
 }
 
diff --git a/xen/include/asm-x86/amd.h b/xen/include/asm-x86/amd.h
index e9867c7823..71fc52924e 100644
--- a/xen/include/asm-x86/amd.h
+++ b/xen/include/asm-x86/amd.h
@@ -148,4 +148,7 @@ extern s8 opt_allow_unsafe;
 void fam10h_check_enable_mmcfg(void);
 void check_enable_amd_mmconf_dmi(void);
 
+extern bool amd_acpi_c1e_quirk;
+void amd_disable_c1e(void *);
+
 #endif /* __AMD_H__ */
-- 
2.19.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 7/7] amd/pvh: enable ACPI C1E disable quirk on PVH Dom0

[Wei Liu]

On Fri, Oct 19, 2018 at 04:20:21PM +0100, Roger Pau Monne wrote:
> PV Dom0 has a quirk for some AMD processors, where enabling ACPI can
> also enable C1E mode. Apply the same workaround as done on PV for a
> PVH Dom0, which consist on trapping accesses to the SMI command IO
> port and disabling C1E if ACPI is enabled.
> 
> Reported-by: Jan Beulich <jbeulich@suse.com>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Oops, we raced each other.

See my recently sent

x86/amd: call post outb hook for both PV and HVM

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 7/7] amd/pvh: enable ACPI C1E disable quirk on PVH Dom0

[Jan Beulich]

>>> On 19.10.18 at 17:20, <roger.pau@citrix.com> wrote:
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -1272,6 +1272,24 @@ void svm_host_osvw_init()
>      spin_unlock(&osvw_lock);
>  }
>  
> +static int acpi_c1e_quirk(int dir, unsigned int port, unsigned int bytes,
> +                          uint32_t *val)
> +{
> +    ASSERT(bytes == 1 && port == acpi_smi_cmd);
> +
> +    if ( dir == IOREQ_READ )
> +    {
> +        *val = inb(port);
> +        return X86EMUL_OKAY;
> +    }
> +
> +    outb(*val, port);
> +    if ( *val == acpi_enable_value )
> +       on_each_cpu(amd_disable_c1e, NULL, 1);
> +
> +    return X86EMUL_OKAY;
> +}

Nothing in here nor ...

> @@ -1284,6 +1302,9 @@ static int svm_domain_initialise(struct domain *d)
>  
>      svm_guest_osvw_init(d);
>  
> +    if ( amd_acpi_c1e_quirk )
> +        register_portio_handler(d, acpi_smi_cmd, 1, acpi_c1e_quirk);

... the registration here restricts this to Dom0.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 1/7] x86/pvh: fix TSC mode setup for PVH Dom0

[Jan Beulich]

>>> On 19.10.18 at 17:20, <roger.pau@citrix.com> wrote:
> A PVH Dom0 might use TSC scaling or other HVM specific TSC
> adjustments, so only short-circuit the TSC setup for a classic PV
> Dom0.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 2/7] x86/hvm: introduce a define for the debug output IO port

[Jan Beulich]

>>> On 19.10.18 at 17:20, <roger.pau@citrix.com> wrote:
> No functional change intended.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
with one remark:

> --- a/xen/include/public/arch-x86/xen.h
> +++ b/xen/include/public/arch-x86/xen.h
> @@ -346,6 +346,12 @@ struct xen_arch_domainconfig {
>  #define XEN_CPUID          XEN_EMULATE_PREFIX "cpuid"
>  #endif
>  
> +/*
> + * Debug console IO port, also called "port E9 hack". Each character written
> + * to this IO port will be printed on the hypervisor console.
> + */
> +#define XEN_HVM_DEBUGCONS_IOPORT 0xe9

Perhaps amend the comment with "..., subject to log level
restrictions"? Easily done while committing.

FTR I'm a little uneasy about adding this to the public
interface, but otoh external components already use it as
if it was part of it.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 3/7] x86/pvh: allow PVH Dom0 to use the debug IO port console

[Jan Beulich]

>>> On 19.10.18 at 17:20, <roger.pau@citrix.com> wrote:
> Add an option to allow trapping accesses to IO port 0xe9 for a PVH
> Dom0, so it can print to the HVM debug console. Note this is not
> enabled by default in order to prevent clashes with hardware on the
> system using 0xe9.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 3/7] x86/pvh: allow PVH Dom0 to use the debug IO port console

[Andrew Cooper]

On 19/10/18 16:20, Roger Pau Monne wrote:
> Add an option to allow trapping accesses to IO port 0xe9 for a PVH
> Dom0, so it can print to the HVM debug console. Note this is not
> enabled by default in order to prevent clashes with hardware on the
> system using 0xe9.
>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> ---
> Changes since v1:
>  - Use a define for 0xe9.
>  - Expand 'List of' in the Xen command doc.

As said during v1, make this unconditional and do away the command line
option.

Noone with a PVH dom0 is going to have a real CoZet Info PC radio on an
ISA bus.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 3/7] x86/pvh: allow PVH Dom0 to use the debug IO port console

[Jan Beulich]

>>> On 29.10.18 at 17:53, <andrew.cooper3@citrix.com> wrote:
> On 19/10/18 16:20, Roger Pau Monne wrote:
>> Add an option to allow trapping accesses to IO port 0xe9 for a PVH
>> Dom0, so it can print to the HVM debug console. Note this is not
>> enabled by default in order to prevent clashes with hardware on the
>> system using 0xe9.
>>
>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>> ---
>> Cc: Jan Beulich <jbeulich@suse.com>
>> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
>> Cc: Wei Liu <wei.liu2@citrix.com>
>> ---
>> Changes since v1:
>>  - Use a define for 0xe9.
>>  - Expand 'List of' in the Xen command doc.
> 
> As said during v1, make this unconditional and do away the command line
> option.
> 
> Noone with a PVH dom0 is going to have a real CoZet Info PC radio on an
> ISA bus.

Oh, I'm sorry - I had forgotten about that request of yours, and
should hence not have given my R-b.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 7/7] amd/pvh: enable ACPI C1E disable quirk on PVH Dom0

[Roger Pau Monné]

On Mon, Oct 29, 2018 at 08:19:20AM -0600, Jan Beulich wrote:
> >>> On 19.10.18 at 17:20, <roger.pau@citrix.com> wrote:
> > --- a/xen/arch/x86/hvm/svm/svm.c
> > +++ b/xen/arch/x86/hvm/svm/svm.c
> > @@ -1272,6 +1272,24 @@ void svm_host_osvw_init()
> >      spin_unlock(&osvw_lock);
> >  }
> >  
> > +static int acpi_c1e_quirk(int dir, unsigned int port, unsigned int bytes,
> > +                          uint32_t *val)
> > +{
> > +    ASSERT(bytes == 1 && port == acpi_smi_cmd);
> > +
> > +    if ( dir == IOREQ_READ )
> > +    {
> > +        *val = inb(port);
> > +        return X86EMUL_OKAY;
> > +    }
> > +
> > +    outb(*val, port);
> > +    if ( *val == acpi_enable_value )
> > +       on_each_cpu(amd_disable_c1e, NULL, 1);
> > +
> > +    return X86EMUL_OKAY;
> > +}
> 
> Nothing in here nor ...
> 
> > @@ -1284,6 +1302,9 @@ static int svm_domain_initialise(struct domain *d)
> >  
> >      svm_guest_osvw_init(d);
> >  
> > +    if ( amd_acpi_c1e_quirk )
> > +        register_portio_handler(d, acpi_smi_cmd, 1, acpi_c1e_quirk);
> 
> ... the registration here restricts this to Dom0.

Right, I got messed up with the permissions and completely forgot
about limiting to Dom0.

The above needs to have a '&& is_hardware_domain(d)' IMO. I prefer to
do this at hook registration rather than adding such check in the
handler itself.

Thanks. Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 2/7] x86/hvm: introduce a define for the debug output IO port

[Roger Pau Monné]

On Mon, Oct 29, 2018 at 10:51:07AM -0600, Jan Beulich wrote:
> >>> On 19.10.18 at 17:20, <roger.pau@citrix.com> wrote:
> > No functional change intended.
> > 
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> 
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> with one remark:
> 
> > --- a/xen/include/public/arch-x86/xen.h
> > +++ b/xen/include/public/arch-x86/xen.h
> > @@ -346,6 +346,12 @@ struct xen_arch_domainconfig {
> >  #define XEN_CPUID          XEN_EMULATE_PREFIX "cpuid"
> >  #endif
> >  
> > +/*
> > + * Debug console IO port, also called "port E9 hack". Each character written
> > + * to this IO port will be printed on the hypervisor console.
> > + */
> > +#define XEN_HVM_DEBUGCONS_IOPORT 0xe9
> 
> Perhaps amend the comment with "..., subject to log level
> restrictions"? Easily done while committing.

Yes, that seems sensible, feel free to add it during commit.

> FTR I'm a little uneasy about adding this to the public
> interface, but otoh external components already use it as
> if it was part of it.

Given the current usage I think this should be part of the public
headers.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 3/7] x86/pvh: allow PVH Dom0 to use the debug IO port console

[Roger Pau Monné]

On Mon, Oct 29, 2018 at 11:03:37AM -0600, Jan Beulich wrote:
> >>> On 29.10.18 at 17:53, <andrew.cooper3@citrix.com> wrote:
> > On 19/10/18 16:20, Roger Pau Monne wrote:
> >> Add an option to allow trapping accesses to IO port 0xe9 for a PVH
> >> Dom0, so it can print to the HVM debug console. Note this is not
> >> enabled by default in order to prevent clashes with hardware on the
> >> system using 0xe9.
> >>
> >> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> >> ---
> >> Cc: Jan Beulich <jbeulich@suse.com>
> >> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> >> Cc: Wei Liu <wei.liu2@citrix.com>
> >> ---
> >> Changes since v1:
> >>  - Use a define for 0xe9.
> >>  - Expand 'List of' in the Xen command doc.
> > 
> > As said during v1, make this unconditional and do away the command line
> > option.
> > 
> > Noone with a PVH dom0 is going to have a real CoZet Info PC radio on an
> > ISA bus.
> 
> Oh, I'm sorry - I had forgotten about that request of yours, and
> should hence not have given my R-b.

I'm misread Andy's comment, will resend with the debug port always on.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH v2 7/7] amd/pvh: enable ACPI C1E disable quirk on PVH Dom0

[Jan Beulich]

>>> On 30.10.18 at 12:00, <roger.pau@citrix.com> wrote:
> On Mon, Oct 29, 2018 at 08:19:20AM -0600, Jan Beulich wrote:
>> >>> On 19.10.18 at 17:20, <roger.pau@citrix.com> wrote:
>> > --- a/xen/arch/x86/hvm/svm/svm.c
>> > +++ b/xen/arch/x86/hvm/svm/svm.c
>> > @@ -1272,6 +1272,24 @@ void svm_host_osvw_init()
>> >      spin_unlock(&osvw_lock);
>> >  }
>> >  
>> > +static int acpi_c1e_quirk(int dir, unsigned int port, unsigned int bytes,
>> > +                          uint32_t *val)
>> > +{
>> > +    ASSERT(bytes == 1 && port == acpi_smi_cmd);
>> > +
>> > +    if ( dir == IOREQ_READ )
>> > +    {
>> > +        *val = inb(port);
>> > +        return X86EMUL_OKAY;
>> > +    }
>> > +
>> > +    outb(*val, port);
>> > +    if ( *val == acpi_enable_value )
>> > +       on_each_cpu(amd_disable_c1e, NULL, 1);
>> > +
>> > +    return X86EMUL_OKAY;
>> > +}
>> 
>> Nothing in here nor ...
>> 
>> > @@ -1284,6 +1302,9 @@ static int svm_domain_initialise(struct domain *d)
>> >  
>> >      svm_guest_osvw_init(d);
>> >  
>> > +    if ( amd_acpi_c1e_quirk )
>> > +        register_portio_handler(d, acpi_smi_cmd, 1, acpi_c1e_quirk);
>> 
>> ... the registration here restricts this to Dom0.
> 
> Right, I got messed up with the permissions and completely forgot
> about limiting to Dom0.
> 
> The above needs to have a '&& is_hardware_domain(d)' IMO. I prefer to
> do this at hook registration rather than adding such check in the
> handler itself.

Of course. No point to register a handler that does nothing.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel