[PATCH v3 1/3] hw/xen/xen_pt: Save back data only for declared registers

Marek Marczykowski-Górecki posted 3 patches 1 year, 9 months ago
Maintainers: Stefano Stabellini <sstabellini@kernel.org>, Anthony PERARD <anthony@xenproject.org>, Paul Durrant <paul@xen.org>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>
[PATCH v3 1/3] hw/xen/xen_pt: Save back data only for declared registers
Posted by Marek Marczykowski-Górecki 1 year, 9 months ago
Call pci_default_write_config() only after resolving any handlers from
XenPTRegInfo structures, and only with a value updated with those
handlers. This is important for two reasons:
1. XenPTRegInfo has ro_mask which needs to be enforced - Xen-specific
   hooks do that on their own (especially xen_pt_*_reg_write()).
2. Not setting value early allows hooks to see the old value too.

If it would be only about the first point, setting PCIDevice.wmask would
probably be sufficient, but given the second point, change those
writes.

Relevant handlers already save data back to the emulated registers
space, call the pci_default_write_config() only for its side effects.

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
v3:
 - use emulated register value for pci_default_write_config() call, not
   the one for writting back to the hardware
 - greatly simplify the patch by calling pci_default_write_config() on
   the whole value
v2:
 - rewrite commit message, previous one was very misleading
 - fix loop saving register values
 - fix int overflow when calculating write mask
---
 hw/xen/xen_pt.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
index 3635d1b..5f12d3c 100644
--- a/hw/xen/xen_pt.c
+++ b/hw/xen/xen_pt.c
@@ -311,7 +311,6 @@ static void xen_pt_pci_write_config(PCIDevice *d, uint32_t addr,
     }
 
     memory_region_transaction_begin();
-    pci_default_write_config(d, addr, val, len);
 
     /* adjust the read and write value to appropriate CFC-CFF window */
     read_val <<= (addr & 3) << 3;
@@ -397,6 +396,12 @@ static void xen_pt_pci_write_config(PCIDevice *d, uint32_t addr,
     /* need to shift back before passing them to xen_host_pci_set_block. */
     val >>= (addr & 3) << 3;
 
+    /* Call default handler for its side effects only, with value already
+     * written by specific handlers. */
+    pci_default_write_config(d, addr,
+                             pci_default_read_config(d, addr, len),
+                             len);
+
     memory_region_transaction_commit();
 
 out:
-- 
git-series 0.9.1

Re: [PATCH v3 1/3] hw/xen/xen_pt: Save back data only for declared registers
Posted by Anthony PERARD 1 year, 5 months ago
On Mon, May 06, 2024 at 02:33:20AM +0200, Marek Marczykowski-Górecki wrote:
> Call pci_default_write_config() only after resolving any handlers from
> XenPTRegInfo structures, and only with a value updated with those
> handlers. This is important for two reasons:
> 1. XenPTRegInfo has ro_mask which needs to be enforced - Xen-specific
>    hooks do that on their own (especially xen_pt_*_reg_write()).
> 2. Not setting value early allows hooks to see the old value too.
> 
> If it would be only about the first point, setting PCIDevice.wmask would
> probably be sufficient, but given the second point, change those
> writes.
> 
> Relevant handlers already save data back to the emulated registers
> space, call the pci_default_write_config() only for its side effects.
> 
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> ---
> v3:
>  - use emulated register value for pci_default_write_config() call, not
>    the one for writting back to the hardware
>  - greatly simplify the patch by calling pci_default_write_config() on
>    the whole value
> v2:
>  - rewrite commit message, previous one was very misleading
>  - fix loop saving register values
>  - fix int overflow when calculating write mask

Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>

Thanks,

-- 

Anthony Perard | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech