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

[Daniel De Graaf <dgdegra@tycho.nsa.gov>]

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