Re: [PATCH] PCI: quirks: Fix register location for UPDCR

["Raj, Ashok" <ashok.raj@intel.com>]

Hi Alex

On Wed, Sep 18, 2019 at 04:04:54PM -0600, Alex Williamson wrote:
> > > > >>
> > > > >> Signed-off-by: Steffen Liebergeld <steffen.liebergeld@kernkonzept.com>    
> > > > > 
> > > > > You may also like to add:
> > > > > 
> > > > > Fixes: d99321b63b1f ("PCI: Enable quirks for PCIe ACS on Intel PCH root ports")
> > > > > Reviewed-by: Andrew Murray <andrew.murray@arm.com>
> > > > > 
> > > > > As well as CC'ing stable.    
> > > > 
> > > > Ok. Thank you.
> > > >     
> > > > > I guess the side effect of this bug is that we claim to have peer
> > > > > isolation when we do not. This fix ensures that we get the advertised
> > > > > isolation.    
> > > > Yes, that is also my understanding. Should I explain that in the commit
> > > > message?    
> > > 
> > > I think something similar to that would be helpful.  
> > 
> > This is unfortunate, but my initial impression is that this may have
> > just been a typo that slipped by everyone.  It's difficult to actually
> > test for isolation.  Maybe someone from Intel could review this.  Also,
> > Steffen discussed this with me prior to posting and I believe this is
> > untested, so while trivial from inspection, it would be preferable to
> > know that some sample of hardware doesn't fall over as a result.
> 
> I've looked at 4 different systems, two 6-series (desktop + laptop), an
> 8-series desktop, and an X79 workstation.  None of the 6/8 series even
> enter the branch where we read the UPDCR register, the value read from
> the BSPR register doesn't require it.  In the case of the X79, using
> 0x1014 for UPDCR, the value read from the register is zero so code
> would not proceed into the inner branch to write the register, but
> using the current 0x1114 address, we read a non-zero value and changing
> it does stick on re-read.  Neither address is defined in the public
> specs for this chipset, we're basing the information on word of mouth
> and ack from Intel as noted in commit 1a30fd0dba77.
> 
> So of these systems, if 0x1014 is the correct UPDCR register address,
> nothing actually changes with respect to isolation other than we're not
> changing the value in mystery register 0x1114.  Intel?  Thanks,

I have asked the chipset folks if the spec has a typo.. waiting for
an answer..