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

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

El 29/04/15 a les 2.38, Andrew Cooper ha escrit:
> On 28/04/15 16:44, Roger Pau Monne wrote:
>> 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 v1:
>>   - Dinamically allocate PVH Dom0 IO bitmap if needed.
>>   - Drop cast from construct_vmcs when writting 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/domain_build.c      | 13 +++++++++++++
>>   xen/arch/x86/hvm/hvm.c           | 21 ++++++++++++++++++++-
>>   xen/arch/x86/hvm/svm/vmcb.c      |  3 ++-
>>   xen/arch/x86/hvm/vmx/vmcs.c      |  5 +++--
>>   xen/arch/x86/setup.c             |  2 ++
>>   xen/include/asm-x86/hvm/domain.h |  2 ++
>>   xen/include/asm-x86/setup.h      |  1 +
>>   7 files changed, 43 insertions(+), 4 deletions(-)
>>
>> diff --git a/xen/arch/x86/domain_build.c b/xen/arch/x86/domain_build.c
>> index 378e650..5283a21 100644
>> --- a/xen/arch/x86/domain_build.c
>> +++ b/xen/arch/x86/domain_build.c
>> @@ -1635,6 +1635,19 @@ out:
>>       return rc;
>>   }
>>   +void __init setup_io_bitmap(struct domain *d)
>> +{
>> +    int i;
> 
> unsigned
> 
>> +
>> +    if ( is_pvh_domain(d) )
>> +    {
>> +        for ( i = 0; i < 0x10000; i++ )
>> +            /* NB: 0xcf8 has special treatment so we need to trap it. */
> 
> Why?  (and irrespective of my question, cf8 expects a 4 byte access, and
> surely cfc would need similar treatment?)

0xcfc-0xcff is already added to ioports_deny_access in construct_dom0. I
have no idea why 0xcf8 needs this special treatment, but Linux PVH fails
to enumerate PCI devices if Xen is not set to trap accesses to 0xcf8
(FreeBSD seems to be fine, either with 0xcf8 trapped or not).

> 
>> +            if ( ioports_access_permitted(d, i, i) && i != 0xcf8 )
>> +                __clear_bit(i, d->arch.hvm_domain.io_bitmap);
> 
> I still think that there must be a more efficient way of doing this loop.

I've changed it to use rangeset_report_ranges.

>> +    }
>> +}
>> +
>>   /*
>>    * Local variables:
>>    * mode: C
>> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
>> index bfde380..450edc3 100644
>> --- a/xen/arch/x86/hvm/hvm.c
>> +++ b/xen/arch/x86/hvm/hvm.c
>> @@ -78,9 +78,10 @@ integer_param("hvm_debug", opt_hvm_debug_level);
>>     struct hvm_function_table hvm_funcs __read_mostly;
>>   +#define HVM_IOBITMAP_SIZE   (3*PAGE_SIZE/BYTES_PER_LONG)
>>   /* I/O permission bitmap is globally shared by all HVM guests. */
>>   unsigned long __attribute__ ((__section__ (".bss.page_aligned")))
>> -    hvm_io_bitmap[3*PAGE_SIZE/BYTES_PER_LONG];
>> +    hvm_io_bitmap[HVM_IOBITMAP_SIZE];
>>     /* Xen command-line option to enable HAP */
>>   static bool_t __initdata opt_hap_enabled = 1;
>> @@ -1484,6 +1485,22 @@ 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 = xzalloc_array(unsigned long,
>> +                                                     HVM_IOBITMAP_SIZE);
> 
> This is an unnecessarily large alignment.  The bitmap only needs page
> alignment.

Ack, will be fixed in new version.

Roger.

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