Re: [Xen-devel] [PATCH RFC] xen/pvh: use a custom IO bitmap for PVH hardware domains

["Tian, Kevin" <kevin.tian@xxxxxxxxx>]

> From: Roger Pau Monne [mailto:roger.pau@xxxxxxxxxx]
> Sent: Wednesday, April 08, 2015 8:57 PM
> 
> 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.
> 
> Also add the IO ports of the serial console used by Xen to the list of not
> accessible IO ports.
> 
> Signed-off-by: Roger Pau Monnà <roger.pau@xxxxxxxxxx>
> Cc: Jan Beulich <jbeulich@xxxxxxxx>
> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Cc: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
> Cc: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>
> Cc: Aravind Gopalakrishnan <Aravind.Gopalakrishnan@xxxxxxx>
> Cc: Jun Nakajima <jun.nakajima@xxxxxxxxx>
> Cc: Eddie Dong <eddie.dong@xxxxxxxxx>
> Cc: Kevin Tian <kevin.tian@xxxxxxxxx>
> ---
>  xen/arch/x86/domain_build.c      | 10 ++++++++++
>  xen/arch/x86/hvm/hvm.c           | 11 +++++++++++
>  xen/arch/x86/hvm/svm/vmcb.c      |  3 ++-
>  xen/arch/x86/hvm/vmx/vmcs.c      |  6 ++++--
>  xen/arch/x86/hvm/vmx/vmx.c       |  1 +
>  xen/drivers/char/ns16550.c       | 10 ++++++++++
>  xen/include/asm-x86/hvm/domain.h |  2 ++
>  xen/include/asm-x86/hvm/hvm.h    |  1 +
>  xen/include/xen/serial.h         |  4 ++++
>  9 files changed, 45 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/x86/domain_build.c b/xen/arch/x86/domain_build.c
> index e5c845c..d0365fe 100644
> --- a/xen/arch/x86/domain_build.c
> +++ b/xen/arch/x86/domain_build.c
> @@ -22,6 +22,7 @@
>  #include <xen/compat.h>
>  #include <xen/libelf.h>
>  #include <xen/pfn.h>
> +#include <xen/serial.h>
>  #include <asm/regs.h>
>  #include <asm/system.h>
>  #include <asm/io.h>
> @@ -1541,6 +1542,11 @@ int __init construct_dom0(
>      rc |= ioports_deny_access(d, 0x40, 0x43);
>      /* PIT Channel 2 / PC Speaker Control. */
>      rc |= ioports_deny_access(d, 0x61, 0x61);
> +    /* Serial console. */
> +    if ( uart_ioport1 != 0 )
> +        rc |= ioports_deny_access(d, uart_ioport1, uart_ioport1 + 7);
> +    if ( uart_ioport2 != 0 )
> +        rc |= ioports_deny_access(d, uart_ioport2, uart_ioport2 + 7);

would this be better in actual serial driver?

>      /* ACPI PM Timer. */
>      if ( pmtmr_ioport )
>          rc |= ioports_deny_access(d, pmtmr_ioport, pmtmr_ioport + 3);
> @@ -1618,6 +1624,10 @@ int __init construct_dom0(
> 
>          pvh_map_all_iomem(d, nr_pages);
>          pvh_setup_e820(d, nr_pages);
> +
> +        for ( i = 0; i < 0x10000; i++ )
> +            if ( ioports_access_permitted(d, i, i) )
> +                __clear_bit(i, hvm_hw_io_bitmap);
>      }
> 
>      if ( d->domain_id == hardware_domid )
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index 3ff87c6..6de89b2 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -82,6 +82,10 @@ struct hvm_function_table hvm_funcs __read_mostly;
>  unsigned long __attribute__ ((__section__ (".bss.page_aligned")))
>      hvm_io_bitmap[3*PAGE_SIZE/BYTES_PER_LONG];
> 
> +/* I/O permission bitmap for HVM hardware domain */
> +unsigned long __attribute__ ((__section__ (".bss.page_aligned")))
> +    hvm_hw_io_bitmap[3*PAGE_SIZE/BYTES_PER_LONG];
> +
>  /* Xen command-line option to enable HAP */
>  static bool_t __initdata opt_hap_enabled = 1;
>  boolean_param("hap", opt_hap_enabled);
> @@ -162,6 +166,7 @@ static int __init hvm_enable(void)
>       * often used for I/O delays, but the vmexits simply slow things down).
>       */
>      memset(hvm_io_bitmap, ~0, sizeof(hvm_io_bitmap));
> +    memset(hvm_hw_io_bitmap, ~0, sizeof(hvm_hw_io_bitmap));
>      if ( hvm_port80_allowed )
>          __clear_bit(0x80, hvm_io_bitmap);
>      __clear_bit(0xed, hvm_io_bitmap);
> @@ -1484,6 +1489,12 @@ 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 = hvm_hw_io_bitmap;
> +    else
> +        d->arch.hvm_domain.io_bitmap = hvm_io_bitmap;
> +

if we want to support multiple PVH hardware domains w/ different
permissions, using global array is not correct here.

>      if ( is_pvh_domain(d) )
>      {
>          register_portio_handler(d, 0, 0x10003, handle_pvh_io);
> diff --git a/xen/arch/x86/hvm/svm/vmcb.c b/xen/arch/x86/hvm/svm/vmcb.c
> index 21292bb..eb2176b 100644
> --- a/xen/arch/x86/hvm/svm/vmcb.c
> +++ b/xen/arch/x86/hvm/svm/vmcb.c
> @@ -72,6 +72,7 @@ static int construct_vmcb(struct vcpu *v)
>  {
>      struct arch_svm_struct *arch_svm = &v->arch.hvm_svm;
>      struct vmcb_struct *vmcb = arch_svm->vmcb;
> +    struct domain *d = v->domain;
> 
>      vmcb->_general1_intercepts =
>          GENERAL1_INTERCEPT_INTR        |
> GENERAL1_INTERCEPT_NMI         |
> @@ -118,7 +119,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  =
> (u64)virt_to_maddr(d->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 d614638..3080979 100644
> --- a/xen/arch/x86/hvm/vmx/vmcs.c
> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
> @@ -986,8 +986,10 @@ 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((char *)d->arch.hvm_domain.io_bitmap +
> 0));
> +    __vmwrite(IO_BITMAP_B,
> +              virt_to_maddr((char *)d->arch.hvm_domain.io_bitmap +
> PAGE_SIZE));
> 
>      if ( cpu_has_vmx_virtual_intr_delivery )
>      {
> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index 2ac1492..d5ccdce 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -3118,6 +3118,7 @@ void vmx_vmexit_handler(struct cpu_user_regs
> *regs)
>              uint16_t port = (exit_qualification >> 16) & 0xFFFF;
>              int bytes = (exit_qualification & 0x07) + 1;
>              int dir = (exit_qualification & 0x08) ? IOREQ_READ :
> IOREQ_WRITE;
> +
>              if ( handle_pio(port, bytes, dir) )
>                  update_guest_eip(); /* Safe: IN, OUT */
>          }
> diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
> index d443880..b382e72 100644
> --- a/xen/drivers/char/ns16550.c
> +++ b/xen/drivers/char/ns16550.c
> @@ -44,6 +44,9 @@ static char __initdata opt_com2[30] = "";
>  string_param("com1", opt_com1);
>  string_param("com2", opt_com2);
> 
> +unsigned int uart_ioport1 = 0;
> +unsigned int uart_ioport2 = 0;
> +
>  static struct ns16550 {
>      int baud, clock_hz, data_bits, parity, stop_bits, fifo_size, irq;
>      u64 io_base;   /* I/O port or memory-mapped I/O address. */
> @@ -1121,6 +1124,13 @@ void __init ns16550_init(int index, struct
> ns16550_defaults *defaults)
>      uart->reg_shift = 0;
> 
>      ns16550_parse_port_config(uart, (index == 0) ? opt_com1 : opt_com2);
> +    if ( uart->baud != 0 && uart->io_base < 0x10000 )
> +    {
> +        if ( index == 0 )
> +            uart_ioport1 = uart->io_base;
> +        else
> +            uart_ioport2 = uart->io_base;
> +    }
>  }
> 
>  #ifdef HAS_DEVICE_TREE
> diff --git a/xen/include/asm-x86/hvm/domain.h
> b/xen/include/asm-x86/hvm/domain.h
> index 0702bf5..d002954 100644
> --- a/xen/include/asm-x86/hvm/domain.h
> +++ b/xen/include/asm-x86/hvm/domain.h
> @@ -143,6 +143,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 0dc909b..55e432e 100644
> --- a/xen/include/asm-x86/hvm/hvm.h
> +++ b/xen/include/asm-x86/hvm/hvm.h
> @@ -213,6 +213,7 @@ extern struct hvm_function_table hvm_funcs;
>  extern bool_t hvm_enabled;
>  extern bool_t cpu_has_lmsl;
>  extern s8 hvm_port80_allowed;
> +extern unsigned long hvm_hw_io_bitmap[];
> 
>  extern const struct hvm_function_table *start_svm(void);
>  extern const struct hvm_function_table *start_vmx(void);
> diff --git a/xen/include/xen/serial.h b/xen/include/xen/serial.h
> index 71e6ade..bc5fb06 100644
> --- a/xen/include/xen/serial.h
> +++ b/xen/include/xen/serial.h
> @@ -24,6 +24,10 @@ void serial_set_rx_handler(int handle, serial_rx_fn fn);
>  /* Number of characters we buffer for an interrupt-driven transmitter. */
>  extern unsigned int serial_txbufsz;
> 
> +/* UART IO port */
> +extern unsigned int uart_ioport1;
> +extern unsigned int uart_ioport2;
> +
>  struct uart_driver;
> 
>  enum serial_port_state {
> --
> 1.9.5 (Apple Git-50.3)

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