[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v7 1/2] xen/vpci: header: status register handler
On 11/21/23 11:27, Stewart Hildebrand wrote: > On 11/21/23 10:18, Roger Pau Monné wrote: >> On Tue, Nov 21, 2023 at 10:03:01AM -0500, Stewart Hildebrand wrote: >>> On 11/21/23 09:45, Roger Pau Monné wrote: >>>> On Wed, Sep 13, 2023 at 10:35:46AM -0400, Stewart Hildebrand wrote: >>>>> @@ -407,26 +439,25 @@ uint32_t vpci_read(pci_sbdf_t sbdf, unsigned int >>>>> reg, unsigned int size) >>>>> >>>>> /* >>>>> * Perform a maybe partial write to a register. >>>>> - * >>>>> - * Note that this will only work for simple registers, if Xen needs to >>>>> - * trap accesses to rw1c registers (like the status PCI header register) >>>>> - * the logic in vpci_write will have to be expanded in order to correctly >>>>> - * deal with them. >>>>> */ >>>>> static void vpci_write_helper(const struct pci_dev *pdev, >>>>> const struct vpci_register *r, unsigned >>>>> int size, >>>>> unsigned int offset, uint32_t data) >>>>> { >>>>> + uint32_t val = 0; >>>>> + >>>>> ASSERT(size <= r->size); >>>>> >>>>> - if ( size != r->size ) >>>>> + if ( (size != r->size) || r->ro_mask ) >>>>> { >>>>> - uint32_t val; >>>>> - >>>>> val = r->read(pdev, r->offset, r->private); >>>>> + val &= ~r->rw1c_mask; >>>>> data = merge_result(val, data, size, offset); >>>>> } >>>>> >>>>> + data &= ~(r->rsvdz_mask | r->ro_mask); >>>>> + data |= val & r->ro_mask; >>>> >>>> I've been thinking about this, and the way the ro_mask is implemented >>>> (and the way we want to handle ro bits) is the same behavior as RsvdP. >>>> I would suggest to rename the ro_mask to rsvdp_mask and note >>>> that for resilience reasons we will handle RO bits as RsvdP. >>> >>> But the reads behave differently. RO should return the value, RsvdP should >>> return 0 when read (according to the PCIe Base Spec 4.0). >> >> Hm, right, sorry for the wrong suggestion. We should force bits to 0 >> for guests reads, but make sure that for writes the value on the >> hardware is preserved. >> >> So we need the separate mask for RsvdP, which will be handled like >> ro_mask in vpci_write_helper() and like rsvdz_mask in vpci_read(). > > Agreed. The only reason I didn't add RsvdP support in this patch was that it > wasn't needed for the status register... Since RsvdP will be needed for the > command register, I think I may as well implement it as part of this series, > perhaps in a separate patch following this one. Stay tuned for v8. I just remembered that RsvdP would actually be useful for the PCI_STATUS_CAP_LIST bit in the status register. So I'll implement it directly in this patch afterall, not a separate one.
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |