Re: [Qemu-devel] [v10][PATCH 03/10] piix: create host bridge to passthrough

[Kay, Allen M]

> -----Original Message-----
> From: Alex Williamson [mailto:address@hidden
> Sent: Tuesday, February 09, 2016 9:44 AM
> Cc: address@hidden; address@hidden;
> address@hidden; address@hidden;
> address@hidden; address@hidden; Kay, Allen M
> <address@hidden>
> Subject: Re: [Qemu-devel] [v10][PATCH 03/10] piix: create host bridge to
> passthrough
> 
> 
> Tiejun's email bounced, so adding Allen for comment.
>

Tiejun has left Intel.  Shuai Ruan is his replacement (cc'ed).

>  Host bridge registers
> 52h, a4h, and a8h seem to be for versions of IGD prior to SandyBridge or just
> wrong. Even the GMCH (50h) doesn't seem to be necessary for the VM host
> bridge, so I'm thinking about dropping all of these for vfio-based IGD
> assignment.

I think dropping those register access in GMCH should be fine.  52h/a4h/a8h 
were for Ironlake IGD.  50h is now mirrored in IGD.  They can be removed as 
BDW/SKL driver no longer access them.

>  Do we know which guest drivers have specific dependencies on
> which host bridge and LPC bridge registers?  It would even be nice to
> document the standard registers for future maintainability.  Thanks,
> 

BDW/SKL drivers no longer need to access those registers in GMCH, especially in 
Universal Passthrough Mode.  From my perspective,  I'm OK with limiting KVM IGD 
passthrough to SKL+ platforms so that we don't need to add those hacks for KVM 
IGD passthrough.

For older HW, there weren't anything documented.  We added those register 
accesses in Xen/QEMU as we brought up Ironlake/SNB platforms on Xen.

Allen

> 
> On Mon, 8 Feb 2016 20:32:35 -0700
> Alex Williamson <address@hidden> wrote:
> 
> > On Wed, 15 Jul 2015 13:37:43 +0800
> > Tiejun Chen <address@hidden> wrote:
> >
> > > Implement a pci host bridge specific to passthrough. Actually this
> > > just inherits the standard one. And we also just expose a minimal
> > > real host bridge pci configuration subset.
> > >
> > > Signed-off-by: Tiejun Chen <address@hidden>
> > > Acked-by: Michael S. Tsirkin <address@hidden>
> > > ---
> > > v10:
> > >
> > > * Nothing is changed.
> > >
> > > v9:
> > >
> > > * Just rebase on the latest.
> > >
> > >  hw/pci-host/piix.c   | 82
> ++++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  include/hw/i386/pc.h |  2 ++
> > >  2 files changed, 84 insertions(+)
> > >
> > > diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c index
> > > a203d93..7adf645 100644
> > > --- a/hw/pci-host/piix.c
> > > +++ b/hw/pci-host/piix.c
> > > @@ -734,6 +734,87 @@ static const TypeInfo i440fx_info = {
> > >      .class_init    = i440fx_class_init,
> > >  };
> > >
> > > +/* IGD Passthrough Host Bridge. */
> > > +typedef struct {
> > > +    uint8_t offset;
> > > +    uint8_t len;
> > > +} IGDHostInfo;
> > > +
> > > +/* Here we just expose minimal host bridge offset subset. */ static
> > > +const IGDHostInfo igd_host_bridge_infos[] = {
> > > +    {0x08, 2},  /* revision id */
> > > +    {0x2c, 2},  /* sybsystem vendor id */
> > > +    {0x2e, 2},  /* sybsystem id */
> > > +    {0x50, 2},  /* SNB: processor graphics control register */
> > > +    {0x52, 2},  /* processor graphics control register */
> > > +    {0xa4, 4},  /* SNB: graphics base of stolen memory */
> > > +    {0xa8, 4},  /* SNB: base of GTT stolen memory */ };
> >
> > Hi,
> >
> > I'm confused by these last 4 registers, could you please help me
> > understand them?
> >
> > Offset 0x50 is the GMCH register for SandyBridge and newer processors,
> > that makes sense, but I suspect we really want to mask out the GMS
> > field to indicate the size of stolen memory is zero?  Is Xen providing
> > the VM with direct access to host stolen memory or does it have support
> > in the VM BIOS for matching the host address?
> >

Xen does provide 1:1 stolen memory mapping in the guest.  However, I do agree 
with you we should disable stolen memory in the guest by mask out GMS field.  
This will avoid complications involving stolen memory/RMRR support.  The only 
features uses stolen memory are Frame Buffer Compression and PAVP content 
protection.  PAVP won't work in the guest anyways as it requires access to 
ME/HECI device.

> > 0x52 is unknown to me, it's listed as reserved for anything since
> > SandyBridge, does this date back to chipset-based graphics versus the
> > current processor-based graphics?
> >

I believe this is same as 0x50 but on old Ironlake platform.  Xen started IGD 
passthrough support in Ironlake.

> > The comment on 0xa4 suggests it should be the BDSM regiser, but that's
> > at offset 0xb0 for everything since SandyBridge.  0xa4 is the second
> > DWORD of the Top Of Memory (TOM) register.
> >
> > I'm guessing by the description of 0xa8 that it might be the BGSM
> > register, but that's at 0xb4.  0xa8 is the first DWORD of the Top Of
> > Upper Usable DRAM (TOUUD).  Neither TOM or TOUUD seem to make any
> sense
> > to expose to the VM, which really makes me wonder if they're actually
> > used.
> >

TOM (0xa0/0xa4) and TOUUD (0xa8) were used in Ironlake generation of Windows 
driver.  They are no longer needed in BDW/SKL drivers.

> > I'm looking at volume 2 of the processor datasheets here:
> >
> > http://www.intel.com/content/www/us/en/processors/core/core-
> technical-resources.html
> >
> > Am I maybe looking in the wrong place?  Thanks for your time,
> >
> > Alex
> >