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

["Roger Pau Monné" <roger.pau@citrix.com>]

El 10/04/15 a les 3.43, Tian, Kevin ha escrit:
>> From: Roger Pau Monne [mailto:roger.pau@citrix.com]
[...]
>> 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?

Definitely, I'm doing it this way because at the point were the uart is
set the initial domain struct has not even been created, much less
passed to the uart driver code.

I will rework this to be more similar to mmio_ro_ranges, which can be
set from driver code directly.

[...]

>> --- 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.

Isn't there supposed to be only one hardware domain that has access to
the full hardware?

Roger.