All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Roger Pau Monné" <roger.pau@citrix.com>
To: Alexey G <x1917x@gmail.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>,
	Wei Liu <wei.liu2@citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Ian Jackson <ian.jackson@eu.citrix.com>,
	Paul Durrant <paul.durrant@citrix.com>,
	Jan Beulich <jbeulich@suse.com>,
	Anthony Perard <anthony.perard@citrix.com>,
	xen-devel@lists.xenproject.org
Subject: Re: [RFC PATCH 07/12] hvmloader: allocate MMCONFIG area in the MMIO hole + minor code refactoring
Date: Thu, 22 Mar 2018 09:57:16 +0000	[thread overview]
Message-ID: <20180322095716.5y2hdyh6xp7nto3g@MacBook-Pro-de-Roger.local> (raw)
In-Reply-To: <20180322084958.0000571c@gmail.com>

On Thu, Mar 22, 2018 at 08:49:58AM +1000, Alexey G wrote:
> On Wed, 21 Mar 2018 17:15:04 +0000
> Roger Pau Monné <roger.pau@citrix.com> wrote:
> [...]
> >> Above scenario makes it obvious that at least for QEMU the MMIO->PCI
> >> conf translation is a redundant step. Why not to allow specifying
> >> for DM whether it prefers to receive MMCONFIG accesses as native
> >> (MMIO ones) or as translated PCI conf ioreqs?  
> >
> >You are just adding an extra level of complexity to an interface
> >that's fairly simple. You register a PCI device using
> >XEN_DMOP_IO_RANGE_PCI and you get IOREQ_TYPE_PCI_CONFIG ioreqs.
> 
> Yes, and it is still needed as we have two distinct (and not equal)
> interfaces to PCI conf space. Apart from 0..FFh range overlapping they
> can be considered very different interfaces. And whether it is a real
> system or emulated -- we can use either one of these two interfaces or
> both.

The legacy PCI config space accesses and the MCFG config space access
are just different methods of accessing the PCI configuration space,
but the data _must_ be exactly the same. I don't see how a device
would care about where the access to the config space originated.

> For QEMU zero changes are needed to support MMCONFIG MMIO accesses if
> they come as MMIO ioreqs. It's just what its MMCONFIG emulation code
> expects.

As I said many times in this thread, you seem to be focused around
what's best for QEMU only, and this is wrong. The IOREQ interface is
used by QEMU, but it's also used by other device emulators.

I get the feeling that you assume that the correct solution is the one
that involves less changes to Xen and QEMU. This is simply not true.

> Anyway, for (kind of vague) users of the multiple ioreq servers
> capability we can enable MMIO translation to PCI conf ioreqs. Note that
> actually this is an extra step, not forwarding trapped MMCONFIG MMIO
> accesses to the selected device model as is.
>
> >Getting both IOREQ_TYPE_PCI_CONFIG and IOREQ_TYPE_COPY for PCI config
> >space access is misleading.
> 
> These are very different accesses, both in transport and capabilities.
> 
> >In both cases Xen would have to do the MCFG access decoding in order
> >to figure out which IOREQ server will handle the request. At which
> >point the only step that you avoid is the reconstruction of the memory
> >access from the IOREQ_TYPE_PCI_CONFIG which is trivial.
> 
> The "reconstruction of the memory access" you mentioned won't be easy
> actually. The thing is, address_space_read/write is not all what we
> need.
> 
> In order to translate PCI conf ioreqs back to emulated MMIO ops, we
> need to be an involved party, mainly to know where MMCONFIG area is
> located so we can construct the address within its range from BDF.
> This piece of information is destroyed in the process of MMIO ioreq
> translation to PCI conf type.

QEMU certainly knows the position of the MCFG area (because it's the
one that tells Xen about it), so I don't understand your concerns
above.

> The code which parse PCI conf ioreqs in xen-hvm.c doesn't know anything
> about the current emulated MMCONFIG state. The correct way to have this
> info is to participate in its emulation. As we don't participate, we
> have no other way than trying to gain backdoor access to PCIHost fields
> via things like object_resolve_*(). This solution is cumbersome and
> ugly but will work... and may break anytime due to changes in QEMU. 

OK, so you don't want to reconstruct the access, fine.

Then just inject it using pcie_mmcfg_data_{read/write} or some similar
wrapper. My suggestion was just to try to use the easier way to get
this injected into QEMU.

> QEMU maintainers will grin while looking at all this I'm afraid --
> trapped MMIO accesses which are translated to PCI conf accesses which
> in turn translated back to emulated MMIO accesses upon receiving, along
> with tedious attempts to gain access to MMCONFIG-related info as we're
> not invited to the MMCONFIG emulation party.
>
> The more I think about it, the more I like the existing
> map_io_range_to_ioreq_server() approach. :( It works without doing
> anything, no hacks, no new interfaces, both MMCONFIG and CF8/CFC are
> working as expected. There is a problem to make it compatible with
> the specific multiple ioreq servers feature, but providing a new
> dmop/hypercall (which you suggest is a must have thing to trap MMCONFIG
> MMIO to give QEMU only the freedom to tell where it is located) allows
> to solve this problem in any possible way, either MMIO -> PCI conf
> translation or anything else.

I'm sorry, but I'm getting lost.

You complain that using IOREQ_TYPE_PCI_CONFIG is not a good approach
because QEMU needs to know the position of the MCFG area if we want to
reconstruct and forward the MMIO access. And then you are proposing to
use IOREQ_TYPE_COPY which _requires_ QEMU to know the position of the
MCFG area in order to do the decoding of the PCI config space access.

> >> We can still route either ioreq
> >> type to multiple device emulators accordingly.  
> >
> >It's exactly the same that's done for IO space PCI config space
> >addresses. QEMU gets an IOREQ_TYPE_PCI_CONFIG and it replays the IO
> >space access using do_outp and cpu_ioreq_pio.
> 
> ...And it is completely limited to basic PCI conf space. I don't know
> the context of this line in xen-hvm.c:
> 
> val = (1u << 31) | ((req->addr & 0x0f00) << 16) | ((sbdf & 0xffff) << 8)
>        | (req->addr & 0xfc);
> 
> but seems like current QEMU versions do not expect anything similar to
> AMD ECS-style accesses for 0CF8h. It is limited to basic PCI conf only.
> 
> >If you think using IOREQ_TYPE_COPY for MCFG accesses is such a benefit
> >for QEMU, why not just translate the IOREQ_TYPE_PCI_CONFIG into
> >IOREQ_TYPE_COPY in handle_ioreq and dispatch it using
> >cpu_ioreq_move?
> 
> Answered above, we need to somehow have access to the info which don't
> belong to us for this step.

Why not? QEMU tells Xen the position of the MCFG area but then you
complain that QEMU doesn't know the position of the MCFG area?

Roger.

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

  parent reply	other threads:[~2018-03-22  9:57 UTC|newest]

Thread overview: 183+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-12 18:33 [Qemu-devel] [RFC PATCH 00/30] Xen Q35 Bringup patches + support for PCIe Extended Capabilities for passed through devices Alexey Gerasimenko
2018-03-12 18:33 ` Alexey Gerasimenko
2018-03-12 18:33 ` [RFC PATCH 01/12] libacpi: new DSDT ACPI table for Q35 Alexey Gerasimenko
2018-03-12 19:38   ` Konrad Rzeszutek Wilk
2018-03-12 20:10     ` Alexey G
2018-03-12 20:32       ` Konrad Rzeszutek Wilk
2018-03-12 21:19         ` Alexey G
2018-03-13  2:41           ` Tian, Kevin
2018-03-19 12:43   ` Roger Pau Monné
2018-03-19 13:57     ` Alexey G
2018-03-12 18:33 ` [RFC PATCH 02/12] Makefile: build and use new DSDT " Alexey Gerasimenko
2018-03-19 12:46   ` Roger Pau Monné
2018-03-19 14:18     ` Alexey G
2018-03-19 13:07   ` Jan Beulich
2018-03-19 14:10     ` Alexey G
2018-03-12 18:33 ` [RFC PATCH 03/12] hvmloader: add function to query an emulated machine type (i440/Q35) Alexey Gerasimenko
2018-03-13 17:26   ` Wei Liu
2018-03-13 17:58     ` Alexey G
2018-03-13 18:04       ` Wei Liu
2018-03-19 12:56   ` Roger Pau Monné
2018-03-19 16:26     ` Alexey G
2018-03-12 18:33 ` [RFC PATCH 04/12] hvmloader: add ACPI enabling for Q35 Alexey Gerasimenko
2018-03-13 17:26   ` Wei Liu
2018-03-19 13:01   ` Roger Pau Monné
2018-03-19 23:59     ` Alexey G
2018-03-12 18:33 ` [RFC PATCH 05/12] hvmloader: add Q35 DSDT table loading Alexey Gerasimenko
2018-03-19 14:45   ` Roger Pau Monné
2018-03-20  0:15     ` Alexey G
2018-03-12 18:33 ` [RFC PATCH 06/12] hvmloader: add basic Q35 support Alexey Gerasimenko
2018-03-19 15:30   ` Roger Pau Monné
2018-03-19 23:44     ` Alexey G
2018-03-20  9:20       ` Roger Pau Monné
2018-03-20 21:23         ` Alexey G
2018-03-12 18:33 ` [RFC PATCH 07/12] hvmloader: allocate MMCONFIG area in the MMIO hole + minor code refactoring Alexey Gerasimenko
2018-03-19 15:58   ` Roger Pau Monné
2018-03-19 19:49     ` Alexey G
2018-03-20  8:50       ` Roger Pau Monné
2018-03-20  9:25         ` Paul Durrant
2018-03-21  0:58         ` Alexey G
2018-03-21  9:09           ` Roger Pau Monné
2018-03-21  9:36             ` Paul Durrant
2018-03-21 14:35               ` Alexey G
2018-03-21 14:58                 ` Paul Durrant
2018-03-21 14:25             ` Alexey G
2018-03-21 14:54               ` Paul Durrant
2018-03-21 17:41                 ` Alexey G
2018-03-21 15:20               ` Roger Pau Monné
2018-03-21 16:56                 ` Alexey G
2018-03-21 17:06                   ` Paul Durrant
2018-03-22  0:31                     ` Alexey G
2018-03-22  9:04                       ` Jan Beulich
2018-03-22  9:55                         ` Alexey G
2018-03-22 10:06                           ` Paul Durrant
2018-03-22 11:56                             ` Alexey G
2018-03-22 12:09                               ` Jan Beulich
2018-03-22 13:05                                 ` Alexey G
2018-03-22 13:20                                   ` Jan Beulich
2018-03-22 14:34                                     ` Alexey G
2018-03-22 14:42                                       ` Jan Beulich
2018-03-22 15:08                                         ` Alexey G
2018-03-23 13:57                                           ` Paul Durrant
2018-03-23 22:32                                             ` Alexey G
2018-03-26  9:24                                               ` Roger Pau Monné
2018-03-26 19:42                                                 ` Alexey G
2018-03-27  8:45                                                   ` Roger Pau Monné
2018-03-27 15:37                                                     ` Alexey G
2018-03-28  9:30                                                       ` Roger Pau Monné
2018-03-28 11:42                                                         ` Alexey G
2018-03-28 12:05                                                           ` Paul Durrant
2018-03-28 10:03                                                       ` Paul Durrant
2018-03-28 14:14                                                         ` Alexey G
2018-03-21 17:15                   ` Roger Pau Monné
2018-03-21 22:49                     ` Alexey G
2018-03-22  9:29                       ` Paul Durrant
2018-03-22 10:05                         ` Roger Pau Monné
2018-03-22 10:09                           ` Paul Durrant
2018-03-22 11:36                             ` Alexey G
2018-03-22 10:50                         ` Alexey G
2018-03-22  9:57                       ` Roger Pau Monné [this message]
2018-03-22 12:29                         ` Alexey G
2018-03-22 12:44                           ` Roger Pau Monné
2018-03-22 15:31                             ` Alexey G
2018-03-23 10:29                               ` Paul Durrant
2018-03-23 11:38                                 ` Jan Beulich
2018-03-23 13:52                                   ` Paul Durrant
2018-05-29 14:23   ` Jan Beulich
2018-05-29 17:56     ` Alexey G
2018-05-29 18:47       ` Alexey G
2018-05-30  4:32         ` Alexey G
2018-05-30  8:13           ` Jan Beulich
2018-05-31  4:25             ` Alexey G
2018-05-30  8:12         ` Jan Beulich
2018-05-31  5:15           ` Alexey G
2018-06-01  5:30             ` Jan Beulich
2018-06-01 15:53               ` Alexey G
2018-03-12 18:33 ` [RFC PATCH 08/12] libxl: Q35 support (new option device_model_machine) Alexey Gerasimenko
2018-03-13 17:25   ` Wei Liu
2018-03-13 17:32     ` Anthony PERARD
2018-03-19 17:01   ` Roger Pau Monné
2018-03-19 22:11     ` Alexey G
2018-03-20  9:11       ` Roger Pau Monné
2018-03-21 16:27         ` Wei Liu
2018-03-21 17:03           ` Anthony PERARD
2018-03-21 16:25       ` Wei Liu
2018-03-12 18:33 ` [RFC PATCH 09/12] libxl: Xen Platform device support for Q35 Alexey Gerasimenko
2018-03-19 15:05   ` Alexey G
2018-03-21 16:32     ` Wei Liu
2018-03-12 18:33 ` [RFC PATCH 10/12] libacpi: build ACPI MCFG table if requested Alexey Gerasimenko
2018-03-19 17:33   ` Roger Pau Monné
2018-03-19 21:46     ` Alexey G
2018-03-20  9:03       ` Roger Pau Monné
2018-03-20 21:06         ` Alexey G
2018-05-29 14:36   ` Jan Beulich
2018-05-29 18:20     ` Alexey G
2018-03-12 18:33 ` [RFC PATCH 11/12] hvmloader: use libacpi to build MCFG table Alexey Gerasimenko
2018-03-14 17:48   ` Alexey G
2018-03-19 17:49   ` Roger Pau Monné
2018-03-19 21:20     ` Alexey G
2018-03-20  8:58       ` Roger Pau Monné
2018-03-20  9:36       ` Jan Beulich
2018-03-20 20:53         ` Alexey G
2018-03-21  7:36           ` Jan Beulich
2018-05-29 14:46   ` Jan Beulich
2018-05-29 17:26     ` Alexey G
2018-03-12 18:33 ` [RFC PATCH 12/12] docs: provide description for device_model_machine option Alexey Gerasimenko
2018-03-12 18:33 ` [Qemu-devel] [RFC PATCH 13/30] pc/xen: Xen Q35 support: provide IRQ handling for PCI devices Alexey Gerasimenko
2018-03-12 18:33   ` Alexey Gerasimenko
2018-03-14 10:48   ` [Qemu-devel] " Paolo Bonzini
2018-03-14 11:28     ` Alexey G
2018-03-14 11:28       ` Alexey G
2018-03-14 10:48   ` Paolo Bonzini
2018-03-12 18:33 ` [Qemu-devel] [RFC PATCH 14/30] pc/q35: Apply PCI bus BSEL property for Xen PCI device hotplug Alexey Gerasimenko
2018-03-12 18:33   ` Alexey Gerasimenko
2018-03-12 18:34 ` [Qemu-devel] [RFC PATCH 15/30] q35/acpi/xen: Provide ACPI PCI hotplug interface for Xen on Q35 Alexey Gerasimenko
2018-03-12 18:34   ` Alexey Gerasimenko
2018-03-12 18:34 ` [Qemu-devel] [RFC PATCH 16/30] q35/xen: Add Xen platform device support for Q35 Alexey Gerasimenko
2018-03-12 18:34   ` Alexey Gerasimenko
2018-03-12 19:44   ` [Qemu-devel] " Eduardo Habkost
2018-03-12 20:56     ` Alexey G
2018-03-12 20:56       ` Alexey G
2018-03-12 21:44       ` Eduardo Habkost
2018-03-12 21:44       ` [Qemu-devel] " Eduardo Habkost
2018-03-13 23:49         ` Alexey G
2018-03-13 23:49           ` Alexey G
2018-03-12 19:44   ` Eduardo Habkost
2018-03-13  9:24   ` [Qemu-devel] " Daniel P. Berrangé
2018-03-13  9:24     ` Daniel P. Berrangé
2018-03-12 18:34 ` [RFC PATCH 17/30] q35: Fix incorrect values for PCIEXBAR masks Alexey Gerasimenko
2018-03-12 18:34 ` [Qemu-devel] [RFC PATCH 18/30] xen/pt: XenHostPCIDevice: provide functions for PCI Capabilities and PCIe Extended Capabilities enumeration Alexey Gerasimenko
2018-03-12 18:34   ` Alexey Gerasimenko
2018-03-12 18:34 ` [Qemu-devel] [RFC PATCH 19/30] xen/pt: avoid reading PCIe device type and cap version multiple times Alexey Gerasimenko
2018-03-12 18:34   ` Alexey Gerasimenko
2018-03-12 18:34 ` [Qemu-devel] [RFC PATCH 20/30] xen/pt: determine the legacy/PCIe mode for a passed through device Alexey Gerasimenko
2018-03-12 18:34   ` Alexey Gerasimenko
2018-03-12 18:34 ` [Qemu-devel] [RFC PATCH 21/30] xen/pt: Xen PCIe passthrough support for Q35: bypass PCIe topology check Alexey Gerasimenko
2018-03-12 18:34   ` Alexey Gerasimenko
2018-03-12 18:34 ` [Qemu-devel] [RFC PATCH 22/30] xen/pt: add support for PCIe Extended Capabilities and larger config space Alexey Gerasimenko
2018-03-12 18:34   ` Alexey Gerasimenko
2018-03-12 18:34 ` [Qemu-devel] [RFC PATCH 23/30] xen/pt: handle PCIe Extended Capabilities Next register Alexey Gerasimenko
2018-03-12 18:34   ` Alexey Gerasimenko
2018-03-12 18:34 ` [Qemu-devel] [RFC PATCH 24/30] xen/pt: allow to hide PCIe Extended Capabilities Alexey Gerasimenko
2018-03-12 18:34   ` Alexey Gerasimenko
2018-03-12 18:34 ` [Qemu-devel] [RFC PATCH 25/30] xen/pt: add Vendor-specific PCIe Extended Capability descriptor and sizing Alexey Gerasimenko
2018-03-12 18:34   ` Alexey Gerasimenko
2018-03-12 18:34 ` [Qemu-devel] [RFC PATCH 26/30] xen/pt: add fixed-size PCIe Extended Capabilities descriptors Alexey Gerasimenko
2018-03-12 18:34   ` Alexey Gerasimenko
2018-03-12 18:34 ` [Qemu-devel] [RFC PATCH 27/30] xen/pt: add AER PCIe Extended Capability descriptor and sizing Alexey Gerasimenko
2018-03-12 18:34   ` Alexey Gerasimenko
2018-03-12 18:34 ` [Qemu-devel] [RFC PATCH 28/30] xen/pt: add descriptors and size calculation for RCLD/ACS/PMUX/DPA/MCAST/TPH/DPC PCIe Extended Capabilities Alexey Gerasimenko
2018-03-12 18:34   ` Alexey Gerasimenko
2018-03-12 18:34 ` [Qemu-devel] [RFC PATCH 29/30] xen/pt: add Resizable BAR PCIe Extended Capability descriptor and sizing Alexey Gerasimenko
2018-03-12 18:34   ` Alexey Gerasimenko
2018-03-12 18:34 ` [Qemu-devel] [RFC PATCH 30/30] xen/pt: add VC/VC9/MFVC PCIe Extended Capabilities descriptors " Alexey Gerasimenko
2018-03-12 18:34   ` Alexey Gerasimenko
2018-03-13  9:21 ` [Qemu-devel] [RFC PATCH 00/30] Xen Q35 Bringup patches + support for PCIe Extended Capabilities for passed through devices Daniel P. Berrangé
2018-03-13  9:21   ` Daniel P. Berrangé
2018-03-13 11:37   ` Alexey G
2018-03-13 11:37     ` Alexey G
2018-03-13 11:44     ` Daniel P. Berrangé
2018-03-13 11:44     ` Daniel P. Berrangé
2018-03-16 17:34 ` Alexey G
2018-03-16 18:26   ` Stefano Stabellini
2018-03-16 18:36   ` Roger Pau Monné

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180322095716.5y2hdyh6xp7nto3g@MacBook-Pro-de-Roger.local \
    --to=roger.pau@citrix.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=anthony.perard@citrix.com \
    --cc=ian.jackson@eu.citrix.com \
    --cc=jbeulich@suse.com \
    --cc=paul.durrant@citrix.com \
    --cc=sstabellini@kernel.org \
    --cc=wei.liu2@citrix.com \
    --cc=x1917x@gmail.com \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.