On 6/11/25 15:28, Stewart Hildebrand wrote:
> 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.
I'm trying a somewhat different approach for the series for v2, and this
patch will no longer strictly be prerequisite. However, this patch seems
to be a desirable cleanup by itself, so I'll send it independently.
>>> ---
>>> 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.
>
