Re: [PATCH v7 1/2] xen/vpci: header: status register handler

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

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.

Thanks, Roger.