Re: PCIe: SLT attribute mismatch: 0xFF020100 instead of 0x20100

["Michael S. Tsirkin" <mst@redhat.com>]

On Tue, Aug 29, 2023 at 02:14:51PM +0100, Peter Maydell wrote:
> On Tue, 29 Aug 2023 at 12:40, Marcin Juszkiewicz
> <marcin.juszkiewicz@linaro.org> wrote:
> >
> > I am working on aarch64/sbsa-ref machine so people can have virtual
> > machine to test their OS against something reminding standards compliant
> > system.
> >
> > One of tools I use is BSA ACS (Base System Architecture - Architecture
> > Compliance Suite) [1] written by Arm. It runs set of tests to check does
> > system conforms to BSA specification.
> >
> > 1. https://github.com/ARM-software/bsa-acs
> >
> >
> > SBSA-ref goes better and better, yet still we have some issues. One of
> > them is test 822 ("Check Type 1 config header rules") which fails on
> > each PCIe root port device:
> >
> > BDF 0x400 : SLT attribute mismatch: 0xFF020100 instead of 0x20100
> > BDF 0x500 : SLT attribute mismatch: 0xFF030300 instead of 0x30300
> > BDF 0x600 : SLT attribute mismatch: 0xFF040400 instead of 0x40400
> >
> > I reported it as an issue [2] and got response that it may be QEMU
> > fault. My pcie knowledge is not good enough to know where the problem is.
> >
> > 2. https://github.com/ARM-software/bsa-acs/issues/193
> >
> >
> > In the comment Gowtham Siddarth wrote:
> >
> > > Regarding the SLT (Secondary Latency Timer) register, the expected
> > > values align with the ACS specifications, registering as 0. However,
> > > a discrepancy arises in the register's attribute, intended to be set
> > > as Read-Only. Contrary to this intent, the bit field seems to
> > > function as> Read-Write. Ordinarily, when attempting to write to the
> > > register by configuring all bits to 1, the anticipated behaviour
> > > should involve rejecting the write operation, maintaining the value
> > > at 0 to uphold the register's designated Read-Only nature. However,
> > > in this scenario, the write action takes effect, leading to a
> > > transformation of the register's value to FFs. This anomaly could
> > > potentially stem from an issue within the emulator.
> >
> > Does someone know where the problem may be? And how to fix it?
> 
> I don't know enough about PCI to be sure here, but it sounds like
> what you're saying is happening is that the test case writes all-1s
> to some PCI register for the root port device, and expects that where
> some bits in it are defined in the spec as read-only they don't get set?
> 
> Which registers exactly is the test case trying to write in this way ?
> 
> I've cc'd the QEMU PCI maintainers.
> 
> thanks
> -- PMM


yes, this is a bug.


static void pci_init_mask_bridge(PCIDevice *d)
{
    /* PCI_PRIMARY_BUS, PCI_SECONDARY_BUS, PCI_SUBORDINATE_BUS and
       PCI_SEC_LETENCY_TIMER */
    memset(d->wmask + PCI_PRIMARY_BUS, 0xff, 4);


note there's a typo: PCI_SEC_LETENCY_TIMER should be
PCI_SEC_LATENCY_TIMER.

But the express spec says:
	This register does not apply to PCI Express. It must be read-only and hardwired to 00h. For PCI Express to PCI/PCI-X
	Bridges, refer to the [PCIe-to-PCI-PCI-X-Bridge] for requirements for this register.


So, only for the pci express to pci express bridges, and only for new
machine types, we need to override the mask to 0:

	d->wmask[PCI_SEC_LATENCY_TIMER] = 0;


-- 
MST