Re: [Xen-devel] Is: qemu-xen mishandling upper 64-bit BAR compared to qemu-tradWas:Re: Dom0 linux 4.0 + devel/for-linus-4.1 branch: p2m.c:884:d0v0 gfn_to_mfn failed! gfn=ffffffff001ed type:4

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Wed, 10 Jun 2015, Jan Beulich wrote:
> >>> On 10.06.15 at 03:02, <konrad.wilk@xxxxxxxxxx> wrote:
> > The problem is that the XSA120-addendm patch (which does not clear
> > the PCI_COMMAND register anymore), causes an missing functionality in 
> > qemu-xen to be exposed. This missing functionality is implemented in
> > qemu-traditional which is why it works there.
> > 
> > The problem is that qemu-xen for any write to the BAR regions
> > updates them to the hypervisor - but only if the real hardware has
> > them enabled (see pci_update_mappings in pci_default_write_config which
> > is called by xen_pt_pci_write_config). Specifically pci_bar_address
> > checks PCI_COMMAND register for PCI_COMMAND_MEMORY. If it is disabled
> > (so no XSA-120 addendum patch), it returns -1 (default value resulting
> > in no changes in the internal structures). If it is enabled, then
> > it updates the d->config space with the value written by the guest.
> > Once xen_pt_pci_write_config is done it syncs up the changes (if there
> > are any) which results in the QEMU calling hypervisor to update the P2M 
> > mapping.
> 
> There's one fundamental aspect I'm not understanding here:
> pci_update_mappings() / pci_bar_address() look at the guest view
> (or at least they ought to be), and the virtual command register
> starts out as zero. Is the bug perhaps that xen_pt_initfn(), after
> having initialized d->config[] via xen_host_pci_get_block(), leaves
> the command register at its host view value (rather than updating
> it alongside reg_entry->data in xen_pt_config_reg_init(), called
> via xen_pt_config_init()), which would have happened to be zero
> without that XSA-120 addendum?

It seems to me that Jan is right: setting the PCI_COMMAND register to
~PCI_COMMAND_MEMORY could be done at initialization time. Would that
fix the bug?


> It is of course concerning that
> there are two (now clearly mismatching) guest views within qemu
> (and along those lines I also wonder whether the apparent
> duplicate maintenance of MSI and MSI-X state, due to
> pci_default_write_config() calling msi{,x}_write_config(), can do
> any good, or why the code uses pci_default_write_config() but
> not pci_default_read_config()).
> 
> It looks to me as if there was a halfhearted attempt to utilize
> infrastructure already available in qemu when these Xen pieces
> got added, leading to hard to understand issues like the one here.
> I.e. even if we addressed the initialization value issue above,
> there would still be two competing emulation layers potentially
> (and I suppose quite likely) leading to differing register state
> later on. Hence I wonder how many more issues there are (to
> come)...

The integration between the existing qemu-traditional code and the
upstream QEMU code was hard. I am ready to believe there are more than
just a few gaps and I would be happy to take further patches to improve
the situation.

In this specific instance, are you referring to d->config, part of
PCIDevice, and all the XenPTRegInfo instances? If so, I think the reason
for having the latter, is that we need more flexibility, we need
individual masks and read and write functions.  At the same time we
cannot really get rid of d->config.

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