Re: [Xen-devel] [PATCH 11/22] vixen: early initialization of Vixen including shared_info mapping

[Anthony Liguori <anthony@xxxxxxxxxxxxx>]

On Sun, Jan 7, 2018 at 12:23 AM, Roger Pau Monné <roger.pau@xxxxxxxxxx> wrote:
> On Sat, Jan 06, 2018 at 02:54:26PM -0800, Anthony Liguori wrote:
>> From: Anthony Liguori <aliguori@xxxxxxxxxx>
>>
>> We split initialization of Vixen into two parts.  The first part
>> just detects the presence of an HVM hypervisor so that we can
>> figure out whether to modify the e820 table.
>>
>> The later initialization is used to actually map the shared_info
>> structure from the parent hypervisor into Xen.
>>
>> Signed-off-by: Anthony Liguori <aliguori@xxxxxxxxxx>
>> ---
>>  xen/arch/x86/guest/vixen.c        | 45 
>> +++++++++++++++++++++++++++++++++++++++
>>  xen/arch/x86/setup.c              |  5 +++++
>>  xen/include/asm-x86/guest/vixen.h |  4 ++++
>>  3 files changed, 54 insertions(+)
>>
>> diff --git a/xen/arch/x86/guest/vixen.c b/xen/arch/x86/guest/vixen.c
>> index d82e68f..d8466ba 100644
>> --- a/xen/arch/x86/guest/vixen.c
>> +++ b/xen/arch/x86/guest/vixen.c
>> @@ -20,8 +20,53 @@
>>   */
>>
>>  #include <asm/guest/vixen.h>
>> +#include <public/version.h>
>>
>>  static int in_vixen;
>> +static uint8_t global_si_data[4 << 10] __attribute__((aligned(4096)));
>
> The shared_info memory page gfn doesn't need to be populated, by doing
> it like this you are wasting a domain's memory page.

Right, Andy previously gave me this feedback.  It's been on my TODO
but just haven't gotten there.  I'll take a look at the PVShim tree
and see if there's something I can cherry pick for better shared info
handling.

>> +    rc = HYPERVISOR_memory_op(XENMEM_add_to_physmap, &xatp);
>> +    if ( rc < 0 )
>> +        printk("Setting shared info page failed: %ld\n", rc);
>> +
>> +    memset(&global_si->native.evtchn_mask[0], 0x00,
>> +           sizeof(global_si->native.evtchn_mask));
>
> Hm, I'm not sure I like to approach of unmasking everything. IMHO I
> would rather mask everything and unmask them when the guest actually
> binds the event channel. That makes sure that an interrupt will get
> injected when the event channel is unmasked (if there's an event
> pending).

This is done in hvmloader and we discovered that guests rely on it.
See hvmloader/xenbus.c:xenbus_shutdown().

Regards,

Anthony Liguori

> Thanks, Roger.
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxxx
> https://lists.xenproject.org/mailman/listinfo/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel