Re: [Xen-devel] [PATCH v9 11/11] vpci/msix: add MSI-X handlers

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 15.03.18 at 13:01, <roger.pau@xxxxxxxxxx> wrote:
> On Wed, Mar 14, 2018 at 11:04:00AM -0600, Jan Beulich wrote:
>> >>> On 14.03.18 at 15:04, <roger.pau@xxxxxxxxxx> wrote:
>> > --- a/xen/arch/x86/hvm/dom0_build.c
>> > +++ b/xen/arch/x86/hvm/dom0_build.c
>> > @@ -1117,7 +1117,7 @@ int __init dom0_construct_pvh(struct domain *d, 
>> > const module_t *image,
>> >  
>> >      pvh_setup_mmcfg(d);
>> >  
>> > -    panic("Building a PVHv2 Dom0 is not yet supported.");
>> > +    printk("WARNING: PVH is an experimental mode with limited 
>> > functionality\n");
>> >      return 0;
>> >  }
>> 
>> Does this need to be accompanied by a new entry in SUPPORT.md,
>> as PVH Dom0 becomes usable now? Otoh issues with Dom0 support
>> aren't normally security issues.
> 
> There's no section about classic PV Dom0 support, so adding one about
> PVH would feel weird IMO.
> 
> I will prepare something in order to add classic PV and PVH Dom0
> support to the document, but I would rather do this as a separate
> patch.

Well, I've asked a question (also in the hope that others, in
particular George, might voice an opinion).

>> > +void vpci_msix_arch_print(const struct vpci_msix *msix)
>> > +{
>> > +    unsigned int i;
>> > +
>> > +    for ( i = 0; i < msix->max_entries; i++ )
>> > +    {
>> > +        const struct vpci_msix_entry *entry = &msix->entries[i];
>> > +
>> > +        printk("%6u vec=%02x%7s%6s%3sassert%5s%7s dest_id=%lu mask=%u 
>> > pirq: %d\n",
>> > +               i, MASK_EXTR(entry->data, MSI_DATA_VECTOR_MASK),
>> > +               entry->data & MSI_DATA_DELIVERY_LOWPRI ? "lowest" : 
>> > "fixed",
>> > +               entry->data & MSI_DATA_TRIGGER_LEVEL ? "level" : "edge",
>> > +               entry->data & MSI_DATA_LEVEL_ASSERT ? "" : "de",
>> > +               entry->addr & MSI_ADDR_DESTMODE_LOGIC ? "log" : "phys",
>> > +               entry->addr & MSI_ADDR_REDIRECTION_LOWPRI ? "lowest" : 
>> > "fixed",
>> > +               MASK_EXTR(entry->addr, MSI_ADDR_DEST_ID_MASK),
>> > +               entry->masked, entry->arch.pirq);
>> > +        if ( !(i % 50) )
>> 
>> Please use a number such that the compiler can convert this to a
>> shift.
> 
> 64 should be fine I guess.
> 
>> > +            process_pending_softirqs();
>> 
>> Careful - is this valid with a spin lock held? Note how e.g.
>> dump_domains() holds an RCU lock only.
> 
> It works ATM, but I guess there could be issues if at some point the
> softirqs need to use the vpci lock. I will add a pair of unlock/lock
> around it.

Provided that is safe.

Jan


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