[PATCH v7 0/2] xen/pvh: use a custom IO bitmap for PVH hardware domains

[PATCH v7 0/2] xen/pvh: use a custom IO bitmap for PVH hardware domains

[Roger Pau Monne]

The only change in this version is to allow the io bitmap of Dom0 to be 
exchanged if a late hardware domain is used.

Thanks, Roger.

[PATCH v7 1/2] xen/pvh: use a custom IO bitmap for PVH hardware domains

[Roger Pau Monne]

Since a PVH hardware domain has access to the physical hardware create a
custom more permissive IO bitmap. The permissions set on the bitmap are
populated based on the contents of the ioports rangeset.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Cc: Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com>
Cc: Jun Nakajima <jun.nakajima@intel.com>
Cc: Eddie Dong <eddie.dong@intel.com>
Cc: Kevin Tian <kevin.tian@intel.com>
Cc: Daniel De Graaf <dgdegra@tycho.nsa.gov>
---
Changes since v6:
 - Remove unneded check for hardware_domain.
 - Reset Dom0 bitmap if using a late hardware domain.

Changes since v5:
 - Fix build with XSM (CONFIG_LATE_HWDOM).

Changes since v4:
 - Split changes also affecting PV to a separate patch.
 - Use int with __clear_bit.
 - Drop pointless cast in vmcb.c.
 - Make HVM_IOBITMAP_SIZE contain the size of the io bitmap pages in bytes.
 - Make setup_io_bitmap a hardware domain specific function, and allow it to
   work with late hw domain init.

Changes since v3:
 - Add the RTC IO ports to the list of blocked ports.
 - Remove admin_io_okay since it's just a wrapper around
   ioports_access_permitted.

Changes since v2:
 - Add 0xcf8-0xcfb to the range of blocked (trapped) IO ports.
 - Use rangeset_report_ranges in order to iterate over the range of not
   trapped IO ports.
 - Allocate the Dom0 PVH IO bitmap with _xmalloc_array, which allows setting
   the alignment to PAGE_SIZE.
 - Tested with Linux PV/PVH using 3.18 and FreeBSD PVH HEAD.

Changes since v1:
 - Dynamically allocate PVH Dom0 IO bitmap if needed.
 - Drop cast from construct_vmcs when writing the IO bitmap.
 - Create a new function that sets up the bitmap before launching Dom0. This
   is needed because ns16550_endboot is called after construct_dom0.
---
 xen/arch/x86/hvm/hvm.c           | 23 +++++++++++++++++++++--
 xen/arch/x86/hvm/svm/vmcb.c      |  2 +-
 xen/arch/x86/hvm/vmx/vmcs.c      |  5 +++--
 xen/arch/x86/setup.c             | 28 ++++++++++++++++++++++++++++
 xen/common/domain.c              |  3 +++
 xen/include/asm-x86/hvm/domain.h |  2 ++
 xen/include/asm-x86/hvm/hvm.h    |  5 +++++
 xen/include/asm-x86/setup.h      |  1 +
 8 files changed, 64 insertions(+), 5 deletions(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 689e402..6ca8f35 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -77,9 +77,12 @@ integer_param("hvm_debug", opt_hvm_debug_level);
 
 struct hvm_function_table hvm_funcs __read_mostly;
 
-/* I/O permission bitmap is globally shared by all HVM guests. */
+/*
+ * The I/O permission bitmap is globally shared by all HVM guests except
+ * the hardware domain that has a more permissive IO bitmap.
+ */
 unsigned long __attribute__ ((__section__ (".bss.page_aligned")))
-    hvm_io_bitmap[3*PAGE_SIZE/BYTES_PER_LONG];
+    hvm_io_bitmap[HVM_IOBITMAP_SIZE/BYTES_PER_LONG];
 
 /* Xen command-line option to enable HAP */
 static bool_t __initdata opt_hap_enabled = 1;
@@ -1461,6 +1464,20 @@ int hvm_domain_initialise(struct domain *d)
         goto fail1;
     d->arch.hvm_domain.io_handler->num_slot = 0;
 
+    /* Set the default IO Bitmap */
+    if ( is_hardware_domain(d) )
+    {
+        d->arch.hvm_domain.io_bitmap = _xmalloc(HVM_IOBITMAP_SIZE, PAGE_SIZE);
+        if ( d->arch.hvm_domain.io_bitmap == NULL )
+        {
+            rc = -ENOMEM;
+            goto fail1;
+        }
+        memset(d->arch.hvm_domain.io_bitmap, ~0, HVM_IOBITMAP_SIZE);
+    }
+    else
+        d->arch.hvm_domain.io_bitmap = hvm_io_bitmap;
+
     if ( is_pvh_domain(d) )
     {
         register_portio_handler(d, 0, 0x10003, handle_pvh_io);
@@ -1496,6 +1513,8 @@ int hvm_domain_initialise(struct domain *d)
     stdvga_deinit(d);
     vioapic_deinit(d);
  fail1:
+    if ( is_hardware_domain(d) )
+        xfree(d->arch.hvm_domain.io_bitmap);
     xfree(d->arch.hvm_domain.io_handler);
     xfree(d->arch.hvm_domain.params);
  fail0:
diff --git a/xen/arch/x86/hvm/svm/vmcb.c b/xen/arch/x86/hvm/svm/vmcb.c
index 21292bb..10afd44 100644
--- a/xen/arch/x86/hvm/svm/vmcb.c
+++ b/xen/arch/x86/hvm/svm/vmcb.c
@@ -118,7 +118,7 @@ static int construct_vmcb(struct vcpu *v)
         svm_disable_intercept_for_msr(v, MSR_AMD64_LWP_CBADDR);
 
     vmcb->_msrpm_base_pa = (u64)virt_to_maddr(arch_svm->msrpm);
-    vmcb->_iopm_base_pa  = (u64)virt_to_maddr(hvm_io_bitmap);
+    vmcb->_iopm_base_pa = virt_to_maddr(v->domain->arch.hvm_domain.io_bitmap);
 
     /* Virtualise EFLAGS.IF and LAPIC TPR (CR8). */
     vmcb->_vintr.fields.intr_masking = 1;
diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
index 3123706..a714549 100644
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -1032,8 +1032,9 @@ static int construct_vmcs(struct vcpu *v)
     }
 
     /* I/O access bitmap. */
-    __vmwrite(IO_BITMAP_A, virt_to_maddr((char *)hvm_io_bitmap + 0));
-    __vmwrite(IO_BITMAP_B, virt_to_maddr((char *)hvm_io_bitmap + PAGE_SIZE));
+    __vmwrite(IO_BITMAP_A, virt_to_maddr(d->arch.hvm_domain.io_bitmap));
+    __vmwrite(IO_BITMAP_B, virt_to_maddr(d->arch.hvm_domain.io_bitmap) +
+                           PAGE_SIZE);
 
     if ( cpu_has_vmx_virtual_intr_delivery )
     {
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 2b9787a..44e7e2e 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1446,6 +1446,8 @@ void __init noreturn __start_xen(unsigned long mbi_p)
 
     dmi_end_boot();
 
+    setup_io_bitmap(dom0);
+
     system_state = SYS_STATE_active;
 
     domain_unpause_by_systemcontroller(dom0);
@@ -1509,6 +1511,32 @@ int __hwdom_init xen_in_range(unsigned long mfn)
     return 0;
 }
 
+static int __hwdom_init io_bitmap_cb(unsigned long s, unsigned long e,
+                                     void *ctx)
+{
+    struct domain *d = ctx;
+    int i;
+
+    ASSERT(s <= INT_MAX && e <= INT_MAX);
+    for ( i = s; i <= e; i++ )
+        __clear_bit(i, d->arch.hvm_domain.io_bitmap);
+
+    return 0;
+}
+
+void __hwdom_init setup_io_bitmap(struct domain *d)
+{
+    int rc;
+
+    if ( has_hvm_container_domain(d) )
+    {
+        memset(d->arch.hvm_domain.io_bitmap, ~0, HVM_IOBITMAP_SIZE);
+        rc = rangeset_report_ranges(d->arch.ioport_caps, 0, 0x10000,
+                                    io_bitmap_cb, d);
+        BUG_ON(rc);
+    }
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 6803c4d..b0e83f5 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -42,6 +42,7 @@
 #include <xsm/xsm.h>
 #include <xen/trace.h>
 #include <xen/tmem.h>
+#include <asm/setup.h>
 
 /* Linux config option: propageted to domain0 */
 /* xen_processor_pmbits: xen control Cx, Px, ... */
@@ -219,6 +220,8 @@ static int late_hwdom_init(struct domain *d)
     rangeset_swap(d->iomem_caps, dom0->iomem_caps);
 #ifdef CONFIG_X86
     rangeset_swap(d->arch.ioport_caps, dom0->arch.ioport_caps);
+    setup_io_bitmap(d);
+    setup_io_bitmap(dom0);
 #endif
 
     rcu_unlock_domain(dom0);
diff --git a/xen/include/asm-x86/hvm/domain.h b/xen/include/asm-x86/hvm/domain.h
index 0f8b19a..e250791 100644
--- a/xen/include/asm-x86/hvm/domain.h
+++ b/xen/include/asm-x86/hvm/domain.h
@@ -141,6 +141,8 @@ struct hvm_domain {
      */
     uint64_t sync_tsc;
 
+    unsigned long         *io_bitmap;
+
     union {
         struct vmx_domain vmx;
         struct svm_domain svm;
diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h
index 77eeac5..abb3444 100644
--- a/xen/include/asm-x86/hvm/hvm.h
+++ b/xen/include/asm-x86/hvm/hvm.h
@@ -81,6 +81,11 @@ struct hvm_trap {
 };
 
 /*
+ * Size of the IO bitmap.
+ */
+#define HVM_IOBITMAP_SIZE       (3*PAGE_SIZE)
+
+/*
  * The hardware virtual machine (HVM) interface abstracts away from the
  * x86/x86_64 CPU virtualization assist specifics. Currently this interface
  * supports Intel's VT-x and AMD's SVM extensions.
diff --git a/xen/include/asm-x86/setup.h b/xen/include/asm-x86/setup.h
index 08bc23a..381d9f8 100644
--- a/xen/include/asm-x86/setup.h
+++ b/xen/include/asm-x86/setup.h
@@ -32,6 +32,7 @@ int construct_dom0(
     module_t *initrd,
     void *(*bootstrap_map)(const module_t *),
     char *cmdline);
+void setup_io_bitmap(struct domain *d);
 
 unsigned long initial_images_nrpages(nodeid_t node);
 void discard_initial_images(void);
-- 
1.9.5 (Apple Git-50.3)


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[PATCH v7 2/2] xen/pvh: trap access to sensitive IO ports

[Roger Pau Monne]

This is needed so Xen can properly trap 4 byte accesses to 0xcf8 in order to
keep consistency with accesses to 0xcfc.

The access to RTC ports also needs to be trapped in order to keep
consistency, this includes RTC_PORT(0) and RTC_PORT(1) (0x70 and 0x71
respectively).

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
---
Changes since v2:
 - Trap RTC ports.

Changes since v1:
 - Only trap on accesses to 0xcf8.
---
 xen/arch/x86/setup.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 44e7e2e..321500b 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -49,6 +49,7 @@
 #include <xen/cpu.h>
 #include <asm/nmi.h>
 #include <asm/alternative.h>
+#include <asm/mc146818rtc.h>
 
 /* opt_nosmp: If true, secondary processors are ignored. */
 static bool_t __initdata opt_nosmp;
@@ -1534,6 +1535,16 @@ void __hwdom_init setup_io_bitmap(struct domain *d)
         rc = rangeset_report_ranges(d->arch.ioport_caps, 0, 0x10000,
                                     io_bitmap_cb, d);
         BUG_ON(rc);
+        /*
+         * NB: we need to trap accesses to 0xcf8 in order
+         * to intercept 4 byte accesses, that need to be
+         * handled by Xen in order to keep consistency.
+         * Access to 1 byte RTC ports also needs to be
+         * trapped in order to keep consistency.
+         */
+        __set_bit(0xcf8, d->arch.hvm_domain.io_bitmap);
+        __set_bit(RTC_PORT(0), d->arch.hvm_domain.io_bitmap);
+        __set_bit(RTC_PORT(1), d->arch.hvm_domain.io_bitmap);
     }
 }
 
-- 
1.9.5 (Apple Git-50.3)


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH v7 1/2] xen/pvh: use a custom IO bitmap for PVH hardware domains

[Jan Beulich]

>>> On 20.05.15 at 12:11, <roger.pau@citrix.com> wrote:
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -42,6 +42,7 @@
>  #include <xsm/xsm.h>
>  #include <xen/trace.h>
>  #include <xen/tmem.h>
> +#include <asm/setup.h>

This caused the ARM build to fail. Instead of once again reverting I
applied the trivial fix eab0647587 without asking for permission by
you ARM maintainers - I hope that's okay in a case like this.

But Roger, considering how many issues recent patches of yours
introduced - please apply a little more care.

Jan

Re: [PATCH v7 1/2] xen/pvh: use a custom IO bitmap for PVH hardware domains

[Roger Pau Monné]

El 20/05/15 a les 13.46, Jan Beulich ha escrit:
>>>> On 20.05.15 at 12:11, <roger.pau@citrix.com> wrote:
>> --- a/xen/common/domain.c
>> +++ b/xen/common/domain.c
>> @@ -42,6 +42,7 @@
>>  #include <xsm/xsm.h>
>>  #include <xen/trace.h>
>>  #include <xen/tmem.h>
>> +#include <asm/setup.h>
> 
> This caused the ARM build to fail. Instead of once again reverting I
> applied the trivial fix eab0647587 without asking for permission by
> you ARM maintainers - I hope that's okay in a case like this.

I'm sorry but I'm not sure what's wrong with this chunk, and AFAICT you
seem to have applied it unmodified. The code added in
xen/common/domain.c is protected with #ifdef CONFIG_X86 and ARM has a
setup.h header.

> But Roger, considering how many issues recent patches of yours
> introduced - please apply a little more care.

Yes, and I'm really sorry for that.

Roger.

Re: [PATCH v7 1/2] xen/pvh: use a custom IO bitmap for PVH hardware domains

[Ian Campbell]

On Wed, 2015-05-20 at 12:46 +0100, Jan Beulich wrote:
> >>> On 20.05.15 at 12:11, <roger.pau@citrix.com> wrote:
> > --- a/xen/common/domain.c
> > +++ b/xen/common/domain.c
> > @@ -42,6 +42,7 @@
> >  #include <xsm/xsm.h>
> >  #include <xen/trace.h>
> >  #include <xen/tmem.h>
> > +#include <asm/setup.h>
> 
> This caused the ARM build to fail. Instead of once again reverting I
> applied the trivial fix eab0647587 without asking for permission by
> you ARM maintainers - I hope that's okay in a case like this.

No problem, I'd have deleted it myself rather than commenting it out.
I'm going to push just such a change momentarily, also without waiting
for acks etc.

Ian.

Re: [PATCH v7 1/2] xen/pvh: use a custom IO bitmap for PVH hardware domains

[Jan Beulich]

>>> On 20.05.15 at 14:03, <roger.pau@citrix.com> wrote:
> El 20/05/15 a les 13.46, Jan Beulich ha escrit:
>>>>> On 20.05.15 at 12:11, <roger.pau@citrix.com> wrote:
>>> --- a/xen/common/domain.c
>>> +++ b/xen/common/domain.c
>>> @@ -42,6 +42,7 @@
>>>  #include <xsm/xsm.h>
>>>  #include <xen/trace.h>
>>>  #include <xen/tmem.h>
>>> +#include <asm/setup.h>
>> 
>> This caused the ARM build to fail. Instead of once again reverting I
>> applied the trivial fix eab0647587 without asking for permission by
>> you ARM maintainers - I hope that's okay in a case like this.
> 
> I'm sorry but I'm not sure what's wrong with this chunk, and AFAICT you
> seem to have applied it unmodified. The code added in
> xen/common/domain.c is protected with #ifdef CONFIG_X86 and ARM has a
> setup.h header.

Right, but I suppose you then also looked at my fixup patch, which
I think explains what was wrong?

Jan

Re: [PATCH v7 1/2] xen/pvh: use a custom IO bitmap for PVH hardware domains

[Roger Pau Monné]

El 20/05/15 a les 14.07, Jan Beulich ha escrit:
>>>> On 20.05.15 at 14:03, <roger.pau@citrix.com> wrote:
>> El 20/05/15 a les 13.46, Jan Beulich ha escrit:
>>>>>> On 20.05.15 at 12:11, <roger.pau@citrix.com> wrote:
>>>> --- a/xen/common/domain.c
>>>> +++ b/xen/common/domain.c
>>>> @@ -42,6 +42,7 @@
>>>>  #include <xsm/xsm.h>
>>>>  #include <xen/trace.h>
>>>>  #include <xen/tmem.h>
>>>> +#include <asm/setup.h>
>>>
>>> This caused the ARM build to fail. Instead of once again reverting I
>>> applied the trivial fix eab0647587 without asking for permission by
>>> you ARM maintainers - I hope that's okay in a case like this.
>>
>> I'm sorry but I'm not sure what's wrong with this chunk, and AFAICT you
>> seem to have applied it unmodified. The code added in
>> xen/common/domain.c is protected with #ifdef CONFIG_X86 and ARM has a
>> setup.h header.
> 
> Right, but I suppose you then also looked at my fixup patch, which
> I think explains what was wrong?

Yes, sorry for that, I've understood that you modified the patch in
order to prevent ARM breakage, not that you pushed another on top.

Roger.

Re: [PATCH v7 1/2] xen/pvh: use a custom IO bitmap for PVH hardware domains

[Jan Beulich]

>>> On 20.05.15 at 14:05, <ian.campbell@citrix.com> wrote:
> On Wed, 2015-05-20 at 12:46 +0100, Jan Beulich wrote:
>> >>> On 20.05.15 at 12:11, <roger.pau@citrix.com> wrote:
>> > --- a/xen/common/domain.c
>> > +++ b/xen/common/domain.c
>> > @@ -42,6 +42,7 @@
>> >  #include <xsm/xsm.h>
>> >  #include <xen/trace.h>
>> >  #include <xen/tmem.h>
>> > +#include <asm/setup.h>
>> 
>> This caused the ARM build to fail. Instead of once again reverting I
>> applied the trivial fix eab0647587 without asking for permission by
>> you ARM maintainers - I hope that's okay in a case like this.
> 
> No problem, I'd have deleted it myself rather than commenting it out.
> I'm going to push just such a change momentarily, also without waiting
> for acks etc.

I actually meant to (I commented it out only for checking that
things build afterwards), but then forgot before committing. I'm
sorry for the extra work this causes.

Jan