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

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

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.

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

Thanks, Roger.

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