* [PATCH v6 0/3] xen/pvh: use a custom IO bitmap for PVH hardware domains @ 2015-05-11 14:57 Roger Pau Monne 2015-05-11 14:57 ` [PATCH v6 1/2] " Roger Pau Monne 2015-05-11 14:57 ` [PATCH v6 2/2] xen/pvh: trap access to sensitive IO ports Roger Pau Monne 0 siblings, 2 replies; 10+ messages in thread From: Roger Pau Monne @ 2015-05-11 14:57 UTC (permalink / raw) To: xen-devel Changes in this version include a build fix for XSM and trapping 0xcf8 and the RTC ports for PVH instead of adding them to ioports_deny_access. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v6 1/2] xen/pvh: use a custom IO bitmap for PVH hardware domains 2015-05-11 14:57 [PATCH v6 0/3] xen/pvh: use a custom IO bitmap for PVH hardware domains Roger Pau Monne @ 2015-05-11 14:57 ` Roger Pau Monne 2015-05-13 9:53 ` Jan Beulich 2015-05-11 14:57 ` [PATCH v6 2/2] xen/pvh: trap access to sensitive IO ports Roger Pau Monne 1 sibling, 1 reply; 10+ messages in thread From: Roger Pau Monne @ 2015-05-11 14:57 UTC (permalink / raw) To: xen-devel Cc: Kevin Tian, Jan Beulich, Jun Nakajima, Andrew Cooper, Eddie Dong, Aravind Gopalakrishnan, Suravee Suthikulpanit, Boris Ostrovsky, 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> --- 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 | 24 ++++++++++++++++++++++-- xen/arch/x86/hvm/svm/vmcb.c | 2 +- xen/arch/x86/hvm/vmx/vmcs.c | 5 +++-- xen/arch/x86/setup.c | 29 +++++++++++++++++++++++++++++ xen/common/domain.c | 2 ++ xen/include/asm-x86/hvm/domain.h | 2 ++ xen/include/asm-x86/setup.h | 1 + 7 files changed, 60 insertions(+), 5 deletions(-) diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index 3a09439..ea052c4 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -77,9 +77,13 @@ 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. */ +#define HVM_IOBITMAP_SIZE (3*PAGE_SIZE) +/* + * 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 +1465,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 +1514,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..3b9aee5 100644 --- a/xen/arch/x86/setup.c +++ b/xen/arch/x86/setup.c @@ -1446,6 +1446,9 @@ void __init noreturn __start_xen(unsigned long mbi_p) dmi_end_boot(); + if ( is_hardware_domain(dom0) ) + setup_io_bitmap(dom0); + system_state = SYS_STATE_active; domain_unpause_by_systemcontroller(dom0); @@ -1509,6 +1512,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; + + ASSERT(is_hardware_domain(d)); + if ( has_hvm_container_domain(d) ) + { + 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..b6b0034 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,7 @@ 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); #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/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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v6 1/2] xen/pvh: use a custom IO bitmap for PVH hardware domains 2015-05-11 14:57 ` [PATCH v6 1/2] " Roger Pau Monne @ 2015-05-13 9:53 ` Jan Beulich 2015-05-14 15:27 ` Roger Pau Monné 0 siblings, 1 reply; 10+ messages in thread From: Jan Beulich @ 2015-05-13 9:53 UTC (permalink / raw) To: Roger Pau Monne Cc: Kevin Tian, Suravee Suthikulpanit, Andrew Cooper, Eddie Dong, Aravind Gopalakrishnan, Jun Nakajima, xen-devel, Boris Ostrovsky >>> On 11.05.15 at 16:57, <roger.pau@citrix.com> wrote: > --- a/xen/arch/x86/setup.c > +++ b/xen/arch/x86/setup.c > @@ -1446,6 +1446,9 @@ void __init noreturn __start_xen(unsigned long mbi_p) > > dmi_end_boot(); > > + if ( is_hardware_domain(dom0) ) > + setup_io_bitmap(dom0); Is it indeed possible for is_hardware_domain() to be false for dom0 at this point? > --- 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,7 @@ 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); > #endif Considering that rangesets are getting swapped rather than copied, I think you also need to reset Dom0's I/O bitmap here to the ordinary, non-hardware domain one. Jan ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v6 1/2] xen/pvh: use a custom IO bitmap for PVH hardware domains 2015-05-13 9:53 ` Jan Beulich @ 2015-05-14 15:27 ` Roger Pau Monné 2015-05-15 6:36 ` Jan Beulich 0 siblings, 1 reply; 10+ messages in thread From: Roger Pau Monné @ 2015-05-14 15:27 UTC (permalink / raw) To: Jan Beulich Cc: Kevin Tian, Suravee Suthikulpanit, Andrew Cooper, Eddie Dong, Aravind Gopalakrishnan, Jun Nakajima, xen-devel, Boris Ostrovsky El 13/05/15 a les 11.53, Jan Beulich ha escrit: >>>> On 11.05.15 at 16:57, <roger.pau@citrix.com> wrote: >> --- a/xen/arch/x86/setup.c >> +++ b/xen/arch/x86/setup.c >> @@ -1446,6 +1446,9 @@ void __init noreturn __start_xen(unsigned long mbi_p) >> >> dmi_end_boot(); >> >> + if ( is_hardware_domain(dom0) ) >> + setup_io_bitmap(dom0); > > Is it indeed possible for is_hardware_domain() to be false for dom0 > at this point? No, I will remove this check in the next version. >> --- 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,7 @@ 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); >> #endif > > Considering that rangesets are getting swapped rather than > copied, I think you also need to reset Dom0's I/O bitmap here > to the ordinary, non-hardware domain one. Yes. Would it be fine to memset it and just call setup_io_bitmap on it again, or would you prefer to exchange it with the static one and free it? Roger. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v6 1/2] xen/pvh: use a custom IO bitmap for PVH hardware domains 2015-05-14 15:27 ` Roger Pau Monné @ 2015-05-15 6:36 ` Jan Beulich 2015-05-15 7:34 ` Roger Pau Monné 0 siblings, 1 reply; 10+ messages in thread From: Jan Beulich @ 2015-05-15 6:36 UTC (permalink / raw) To: Roger Pau Monné Cc: Kevin Tian, SuraveeSuthikulpanit, Andrew Cooper, Eddie Dong, Aravind Gopalakrishnan, Jun Nakajima, xen-devel, Boris Ostrovsky >>> On 14.05.15 at 17:27, <roger.pau@citrix.com> wrote: > El 13/05/15 a les 11.53, Jan Beulich ha escrit: >>>>> On 11.05.15 at 16:57, <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> >>> >>> /* Linux config option: propageted to domain0 */ >>> /* xen_processor_pmbits: xen control Cx, Px, ... */ >>> @@ -219,6 +220,7 @@ 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); >>> #endif >> >> Considering that rangesets are getting swapped rather than >> copied, I think you also need to reset Dom0's I/O bitmap here >> to the ordinary, non-hardware domain one. > > Yes. Would it be fine to memset it and just call setup_io_bitmap on it > again, or would you prefer to exchange it with the static one and free it? Following how the rangesets are being treated, simply swapping the two I/O bitmaps would seem to be the right approach here. Jan ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v6 1/2] xen/pvh: use a custom IO bitmap for PVH hardware domains 2015-05-15 6:36 ` Jan Beulich @ 2015-05-15 7:34 ` Roger Pau Monné 2015-05-15 7:42 ` Jan Beulich 0 siblings, 1 reply; 10+ messages in thread From: Roger Pau Monné @ 2015-05-15 7:34 UTC (permalink / raw) To: Jan Beulich Cc: Kevin Tian, SuraveeSuthikulpanit, Andrew Cooper, Eddie Dong, Aravind Gopalakrishnan, Jun Nakajima, xen-devel, Boris Ostrovsky El 15/05/15 a les 8.36, Jan Beulich ha escrit: >>>> On 14.05.15 at 17:27, <roger.pau@citrix.com> wrote: >> El 13/05/15 a les 11.53, Jan Beulich ha escrit: >>>>>> On 11.05.15 at 16:57, <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> >>>> >>>> /* Linux config option: propageted to domain0 */ >>>> /* xen_processor_pmbits: xen control Cx, Px, ... */ >>>> @@ -219,6 +220,7 @@ 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); >>>> #endif >>> >>> Considering that rangesets are getting swapped rather than >>> copied, I think you also need to reset Dom0's I/O bitmap here >>> to the ordinary, non-hardware domain one. >> >> Yes. Would it be fine to memset it and just call setup_io_bitmap on it >> again, or would you prefer to exchange it with the static one and free it? > > Following how the rangesets are being treated, simply swapping > the two I/O bitmaps would seem to be the right approach here. AFAICT this requires adding a new hook in hvm_function_table in order to implement setting the io bitmap for SVM and VMX. I don't have a problem with that, but it's going to need a separate patch. Roger. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v6 1/2] xen/pvh: use a custom IO bitmap for PVH hardware domains 2015-05-15 7:34 ` Roger Pau Monné @ 2015-05-15 7:42 ` Jan Beulich 2015-05-15 20:09 ` Daniel De Graaf 0 siblings, 1 reply; 10+ messages in thread From: Jan Beulich @ 2015-05-15 7:42 UTC (permalink / raw) To: Roger Pau Monné Cc: Kevin Tian, SuraveeSuthikulpanit, Andrew Cooper, Eddie Dong, Aravind Gopalakrishnan, Jun Nakajima, xen-devel, Boris Ostrovsky, dgdegra >>> On 15.05.15 at 09:34, <roger.pau@citrix.com> wrote: > El 15/05/15 a les 8.36, Jan Beulich ha escrit: >>>>> On 14.05.15 at 17:27, <roger.pau@citrix.com> wrote: >>> El 13/05/15 a les 11.53, Jan Beulich ha escrit: >>>>>>> On 11.05.15 at 16:57, <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> >>>>> >>>>> /* Linux config option: propageted to domain0 */ >>>>> /* xen_processor_pmbits: xen control Cx, Px, ... */ >>>>> @@ -219,6 +220,7 @@ 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); >>>>> #endif >>>> >>>> Considering that rangesets are getting swapped rather than >>>> copied, I think you also need to reset Dom0's I/O bitmap here >>>> to the ordinary, non-hardware domain one. >>> >>> Yes. Would it be fine to memset it and just call setup_io_bitmap on it >>> again, or would you prefer to exchange it with the static one and free it? >> >> Following how the rangesets are being treated, simply swapping >> the two I/O bitmaps would seem to be the right approach here. > > AFAICT this requires adding a new hook in hvm_function_table in order to > implement setting the io bitmap for SVM and VMX. I don't have a problem > with that, but it's going to need a separate patch. Right - if there is nothing like that currently, it'll need to be added. Of course you may want to get Daniel de Graaf's (who originally added this non-Dom0 hardware domain code, now Cc-ed) input on the above outline approach before going that route... Jan ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v6 1/2] xen/pvh: use a custom IO bitmap for PVH hardware domains 2015-05-15 7:42 ` Jan Beulich @ 2015-05-15 20:09 ` Daniel De Graaf 2015-05-18 7:12 ` Jan Beulich 0 siblings, 1 reply; 10+ messages in thread From: Daniel De Graaf @ 2015-05-15 20:09 UTC (permalink / raw) To: Jan Beulich, Roger Pau Monné Cc: Kevin Tian, SuraveeSuthikulpanit, Andrew Cooper, Eddie Dong, Aravind Gopalakrishnan, Jun Nakajima, xen-devel, Boris Ostrovsky On 05/15/2015 03:42 AM, Jan Beulich wrote: >>>> On 15.05.15 at 09:34, <roger.pau@citrix.com> wrote: >> El 15/05/15 a les 8.36, Jan Beulich ha escrit: >>>>>> On 14.05.15 at 17:27, <roger.pau@citrix.com> wrote: >>>> El 13/05/15 a les 11.53, Jan Beulich ha escrit: >>>>>>>> On 11.05.15 at 16:57, <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> >>>>>> >>>>>> /* Linux config option: propageted to domain0 */ >>>>>> /* xen_processor_pmbits: xen control Cx, Px, ... */ >>>>>> @@ -219,6 +220,7 @@ 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); >>>>>> #endif >>>>> >>>>> Considering that rangesets are getting swapped rather than >>>>> copied, I think you also need to reset Dom0's I/O bitmap here >>>>> to the ordinary, non-hardware domain one. >>>> >>>> Yes. Would it be fine to memset it and just call setup_io_bitmap on it >>>> again, or would you prefer to exchange it with the static one and free it? >>> >>> Following how the rangesets are being treated, simply swapping >>> the two I/O bitmaps would seem to be the right approach here. >> >> AFAICT this requires adding a new hook in hvm_function_table in order to >> implement setting the io bitmap for SVM and VMX. I don't have a problem >> with that, but it's going to need a separate patch. > > Right - if there is nothing like that currently, it'll need to be added. > Of course you may want to get Daniel de Graaf's (who originally > added this non-Dom0 hardware domain code, now Cc-ed) input on > the above outline approach before going that route... Currently, the only user of this code that I know of is a domain builder which is a PV stub domain, which may impact a trivial swapping operation because dom0->arch.hvm_domain will not be valid. The primary reason why the rangeset_swap calls are not rangeset_dup is because this stub domain does not need access to the hardware after the creation of the hardware domain. I think it seems cleaner for the IO bitmap of domain 0 to also be cleared after the hardware domain is created, but it's not really a requirement to make things work. -- Daniel De Graaf National Security Agency ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v6 1/2] xen/pvh: use a custom IO bitmap for PVH hardware domains 2015-05-15 20:09 ` Daniel De Graaf @ 2015-05-18 7:12 ` Jan Beulich 0 siblings, 0 replies; 10+ messages in thread From: Jan Beulich @ 2015-05-18 7:12 UTC (permalink / raw) To: Daniel De Graaf Cc: Kevin Tian, SuraveeSuthikulpanit, Andrew Cooper, Eddie Dong, Aravind Gopalakrishnan, Jun Nakajima, xen-devel, Boris Ostrovsky, roger.pau >>> On 15.05.15 at 22:09, <dgdegra@tycho.nsa.gov> wrote: > On 05/15/2015 03:42 AM, Jan Beulich wrote: >>>>> On 15.05.15 at 09:34, <roger.pau@citrix.com> wrote: >>> El 15/05/15 a les 8.36, Jan Beulich ha escrit: >>>>>>> On 14.05.15 at 17:27, <roger.pau@citrix.com> wrote: >>>>> El 13/05/15 a les 11.53, Jan Beulich ha escrit: >>>>>>>>> On 11.05.15 at 16:57, <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> >>>>>>> >>>>>>> /* Linux config option: propageted to domain0 */ >>>>>>> /* xen_processor_pmbits: xen control Cx, Px, ... */ >>>>>>> @@ -219,6 +220,7 @@ 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); >>>>>>> #endif >>>>>> >>>>>> Considering that rangesets are getting swapped rather than >>>>>> copied, I think you also need to reset Dom0's I/O bitmap here >>>>>> to the ordinary, non-hardware domain one. >>>>> >>>>> Yes. Would it be fine to memset it and just call setup_io_bitmap on it >>>>> again, or would you prefer to exchange it with the static one and free it? >>>> >>>> Following how the rangesets are being treated, simply swapping >>>> the two I/O bitmaps would seem to be the right approach here. >>> >>> AFAICT this requires adding a new hook in hvm_function_table in order to >>> implement setting the io bitmap for SVM and VMX. I don't have a problem >>> with that, but it's going to need a separate patch. >> >> Right - if there is nothing like that currently, it'll need to be added. >> Of course you may want to get Daniel de Graaf's (who originally >> added this non-Dom0 hardware domain code, now Cc-ed) input on >> the above outline approach before going that route... > > Currently, the only user of this code that I know of is a domain builder > which is a PV stub domain, which may impact a trivial swapping operation > because dom0->arch.hvm_domain will not be valid. The primary reason why > the rangeset_swap calls are not rangeset_dup is because this stub domain > does not need access to the hardware after the creation of the hardware > domain. > > I think it seems cleaner for the IO bitmap of domain 0 to also be cleared > after the hardware domain is created, but it's not really a requirement to > make things work. Hmm - I think the rangeset handling and the I/O bitmap handling ought to be in sync (even if only to avoid future confusion/questions). And yes, considering the case of one of the two being PV and the other PVH is going to be necessary (even if right now only a PV stub domain builder may exist). Jan ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v6 2/2] xen/pvh: trap access to sensitive IO ports 2015-05-11 14:57 [PATCH v6 0/3] xen/pvh: use a custom IO bitmap for PVH hardware domains Roger Pau Monne 2015-05-11 14:57 ` [PATCH v6 1/2] " Roger Pau Monne @ 2015-05-11 14:57 ` Roger Pau Monne 1 sibling, 0 replies; 10+ messages in thread From: Roger Pau Monne @ 2015-05-11 14:57 UTC (permalink / raw) To: xen-devel; +Cc: Andrew Cooper, Jan Beulich, 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 3b9aee5..79593d9 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; @@ -1535,6 +1536,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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2015-05-18 7:12 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-05-11 14:57 [PATCH v6 0/3] xen/pvh: use a custom IO bitmap for PVH hardware domains Roger Pau Monne 2015-05-11 14:57 ` [PATCH v6 1/2] " Roger Pau Monne 2015-05-13 9:53 ` Jan Beulich 2015-05-14 15:27 ` Roger Pau Monné 2015-05-15 6:36 ` Jan Beulich 2015-05-15 7:34 ` Roger Pau Monné 2015-05-15 7:42 ` Jan Beulich 2015-05-15 20:09 ` Daniel De Graaf 2015-05-18 7:12 ` Jan Beulich 2015-05-11 14:57 ` [PATCH v6 2/2] xen/pvh: trap access to sensitive IO ports Roger Pau Monne
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.