Re: [Xen-devel] [RFC PATCH v2 07/26] ARM: GICv3 ITS: introduce host LPI array

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 6 Jan 2017, Andre Przywara wrote:
> >> +/* LPIs on the host always go to a guest, so no struct irq_desc for them. 
> >> */
> >> +union host_lpi {
> >> +    uint64_t data;
> >> +    struct {
> >> +        uint64_t virt_lpi:32;
> >> +        uint64_t dom_id:16;
> >> +        uint64_t vcpu_id:16;
> >> +    };
> >> +};
> > 
> > Just go with a regular struct
> > 
> >     struct host_lpi {
> >         uint32_t virt_lpi;
> >         uint16_t dom_id;
> >         uint16_t vcpu_id;
> >     };
> > 
> > The aarch64 C ABI guarantees the alignments of the fields.
> 
> Yes, I will get rid of the bitfields. But the actual purpose of the
> union is to allow lock-free atomic access. I just see now that I failed
> to document that, sorry!
> 
> We can't afford to have a lock for the actual data here, so the idea was
> to use the naturally atomic access a native data type would give us.
> In case we want to write multiple members, we assemble them in a local
> copy and then write the uint64_t variable into the actual location.
> Similar for reading. Single members can be updated using the members
> directly.
> Since the architecture guarantees atomic access for an aligned memory
> access to/from a GPR, I think this is safe.
> I am not sure we need to use the atomic_<type>_{read,write} accessors
> here? I tried it: the resulting assembly is identical, and the source
> doesn't look too bad either, so I guess I will change them over, just to
> be safe?

Yes, it will also work as documentation


> > It is also possible to end up calling mapti with an inexistent eventid
> > for host_devid. Could that be a problem?
> 
> Not at all. Actually there is no such thing as a "nonexistent event ID",
> because the event ID will be written by the device as the payload to the
> MSI doorbell address, probably because it learned about it by the
> driver. So if we provision an ITTE with an event ID which the device
> will never send, that LPI will just never fire.
> Since Xen (in contrast to the driver in the domain) has no idea how many
> and which MSIs the device will use, we just allocate a bunch of them.
> The upper limit (32 atm) is something we probably need to still think
> about, though.
> I tried to learn a limit from Linux ("nvecs" in its_create_device()
> seems to be the source), but couldn't find anything useful other than 32.
> We will learn about exceeding this as soon as a domain tries to map a
> virtual LPI with an event ID higher than 31, however it's too late to
> fix this then. We can bark when this happens to learn if any device ever
> does this during our testing to get some heuristic data.
> 
> Eventually all boils down to Xen getting more information from Dom0
> about the required number of MSIs. We could then even limit the
> allocation to less than 32, if that helps.

Originally Julien and I thought that Xen should map events up to the
theoretically maximum for each device, but we realized that they were
too many: an MSI-X capable device can generate up to 2048 different
events.

Xen needs to find out the exact number of events for each device. The
information can either be provided by the guest, or the hypervisor needs
to figure it out on its own.

With Julien's PCI Passthrough work, Xen will become able to read the
amount of events a device is capable of generating, so in the long term
this problem should be easy to solve. But Julien's work might land one
or two Xen releases after ITS.

In the meantime, we can extend an existing PHYSDEVOP hypercall or add a
new one. Julien, do you agree?

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