On 6/5/25 05:47, Roger Pau Monné wrote:
> On Sat, May 31, 2025 at 08:53:59AM -0400, Stewart Hildebrand wrote:
>> Since 622bdd962822 ("vpci/header: handle p2m range sets per BAR"), a
>> non-const pdev is no longer needed for error handling in
>> vpci_process_pending(). Const-ify pdev in vpci_process_pending(),
>> defer_map(), and struct vpci_vcpu.
>>
>> Get rid of const-removal workaround in modify_bars().
>>
>> Take the opportunity to remove an unused parameter in defer_map().
>>
>> Signed-off-by: Stewart Hildebrand <stewart.hildebr...@amd.com>
>
> Reviewed-by: Roger Pau Monné <roger....@citrix.com>
Thanks!
> One further simplification below.
>
>> ---
>> This is prerequisite for ("vpci: use separate rangeset for BAR
>> unmapping") in order to call defer_map() with a const pdev.
>> ---
>> xen/drivers/vpci/header.c | 16 ++++------------
>> xen/include/xen/vpci.h | 2 +-
>> 2 files changed, 5 insertions(+), 13 deletions(-)
>>
>> diff --git a/xen/drivers/vpci/header.c b/xen/drivers/vpci/header.c
>> index 1f48f2aac64e..e42c8efa2302 100644
>> --- a/xen/drivers/vpci/header.c
>> +++ b/xen/drivers/vpci/header.c
>> @@ -175,7 +175,7 @@ static void modify_decoding(const struct pci_dev *pdev,
>> uint16_t cmd,
>>
>> bool vpci_process_pending(struct vcpu *v)
>> {
>> - struct pci_dev *pdev = v->vpci.pdev;
>> + const struct pci_dev *pdev = v->vpci.pdev;
>> struct vpci_header *header = NULL;
>> unsigned int i;
>>
>> @@ -283,8 +283,7 @@ static int __init apply_map(struct domain *d, const
>> struct pci_dev *pdev,
>> return rc;
>> }
>>
>> -static void defer_map(struct domain *d, struct pci_dev *pdev,
>> - uint16_t cmd, bool rom_only)
>> +static void defer_map(const struct pci_dev *pdev, uint16_t cmd, bool
>> rom_only)
>> {
>> struct vcpu *curr = current;
>>
>> @@ -308,7 +307,7 @@ static void defer_map(struct domain *d, struct pci_dev
>> *pdev,
>> static int modify_bars(const struct pci_dev *pdev, uint16_t cmd, bool
>> rom_only)
>> {
>> struct vpci_header *header = &pdev->vpci->header;
>> - struct pci_dev *tmp, *dev = NULL;
>> + struct pci_dev *tmp;
>> const struct domain *d;
>> const struct vpci_msix *msix = pdev->vpci->msix;
>> unsigned int i, j;
>> @@ -450,11 +449,6 @@ static int modify_bars(const struct pci_dev *pdev,
>> uint16_t cmd, bool rom_only)
>>
>> if ( tmp == pdev )
>> {
>> - /*
>> - * Need to store the device so it's not constified and
>> defer_map
>> - * can modify it in case of error.
>> - */
>> - dev = tmp;
>> if ( !rom_only )
>
> You can now join this with the previous if, and reduce one level of
> indentation:
>
> if ( tmp == pdev && !rom_only )
> /* comment text */
> continue;
Will do. I'll plan to keep your R-b tag for v2 since this is a trivial
change.
