Re: PVH Dom0 related UART failure

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

On Fri, May 19, 2023 at 10:20:24AM +0200, Jan Beulich wrote:
> On 19.05.2023 09:38, Roger Pau Monné wrote:
> > On Fri, May 19, 2023 at 09:22:58AM +0200, Jan Beulich wrote:
> >> On 18.05.2023 12:34, Roger Pau Monné wrote:
> >>> On Wed, May 17, 2023 at 05:59:31PM -0700, Stefano Stabellini wrote:
> >>>> I have run into another PVH Dom0 issue. I am trying to enable a PVH Dom0
> >>>> test with the brand new gitlab-ci runner offered by Qubes. It is an AMD
> >>>> Zen3 system and we already have a few successful tests with it, see
> >>>> automation/gitlab-ci/test.yaml.
> >>>>
> >>>> We managed to narrow down the issue to a console problem. We are
> >>>> currently using console=com1 com1=115200,8n1,pci,msi as Xen command line
> >>>> options, it works with PV Dom0 and it is using a PCI UART card.
> >>>>
> >>>> In the case of Dom0 PVH:
> >>>> - it works without console=com1
> >>>> - it works with console=com1 and with the patch appended below
> >>>> - it doesn't work otherwise and crashes with this error:
> >>>> https://matrix-client.matrix.org/_matrix/media/r0/download/invisiblethingslab.com/uzcmldIqHptFZuxqsJtviLZK
> >>>
> >>> Jan also noticed this, and we have a ticket for it in gitlab:
> >>>
> >>> https://gitlab.com/xen-project/xen/-/issues/85
> >>>
> >>>> What is the right way to fix it?
> >>>
> >>> I think the right fix is to simply avoid hidden devices from being
> >>> handled by vPCI, in any case such devices won't work propewrly with
> >>> vPCI because they are in use by Xen, and so any cached information by
> >>> vPCI is likely to become stable as Xen can modify the device without
> >>> vPCI noticing.
> >>>
> >>> I think the chunk below should help.  It's not clear to me however how
> >>> hidden devices should be handled, is the intention to completely hide
> >>> such devices from dom0?
> >>
> >> No, Dom0 should still be able to see them in a (mostly) r/o fashion.
> >> Hence my earlier RFC patch making vPCI actually deal with them.
> > 
> > What's the difference between a hidden device and one that's marked RO
> > then?
> 
> pci_hide_device() simply makes the device unavailable for assignment
> (by having it owned by DomXEN). pci_ro_device() additionally adds the
> device to the segment's ro_map, thus protecting its config space from
> Dom0 writes.

But above you mention that hidden devices should be visible to dom0
"in a (mostly) r/o fashion.".

I understand that for RO devices the whole config space of the device
is RO, in which case we should simply avoid using vPCI for them.  We
however likely want to have the BARs of such devices permanently
mapped into the dom0 physmap (if memory decoding is enabled).

But for hidden devices it's not clear to me what needs to be RO, do we
also need to protect the config space from dom0 accesses?

It might be complicated for vPCI to deal with devices that have MSI-X
interrupts in use by Xen for example.  So I would suggest that at
leeat for the time being we don't handle hidden devices with vPCI.

We might want to do similar to RO devices and prevent write access to
the config space for those also.

> And just to clarify - a PCI dev containing a UART isn't "hidden" in the
> above sense, but "r/o" (by virtue of calling pci_ro_device() on it).
> But the issue reported long ago (and now re-discovered by Stefano) is
> common to "r/o" and "hidden" devices (it's the "hidden" aspect that
> counts here, which is common for both).

Indeed, the issue is with any device assigned to dom_xen (or not
assigned to the hardware domain, but accessible by the hardware domain
from vPCI).