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).
