When vpci fails to initialize a extended capability of device, it
just returns an error and vPCI gets disabled for the whole device.
So, add function to hide extended capability when initialization
fails. And remove the failed extended capability handler from vpci
extended capability list.
Signed-off-by: Jiqian Chen <Jiqian.Chen@amd.com>
---
cc: "Roger Pau Monné" <roger.pau@citrix.com>
cc: Andrew Cooper <andrew.cooper3@citrix.com>
cc: Anthony PERARD <anthony.perard@vates.tech>
cc: Michal Orzel <michal.orzel@amd.com>
cc: Jan Beulich <jbeulich@suse.com>
cc: Julien Grall <julien@xen.org>
cc: Stefano Stabellini <sstabellini@kernel.org>
---
v4->v5 changes:
* Modify the hex digits of PCI_EXT_CAP_NEXT_MASK and PCI_EXT_CAP_NEXT to be low case.
* Rename vpci_ext_capability_mask to vpci_ext_capability_hide.
v3->v4 changes:
* Change definition of PCI_EXT_CAP_NEXT to be "#define PCI_EXT_CAP_NEXT(header) (MASK_EXTR(header, PCI_EXT_CAP_NEXT_MASK) & 0xFFCU)" to avoid redundancy.
* Modify the commit message.
* Change vpci_ext_capability_mask() to return error instead of using ASSERT.
* Set the capability ID part to be zero when we need to hide the capability of position 0x100U.
* Add check "if ( !offset )" in vpci_ext_capability_mask().
v2->v3 changes:
* Separated from the last version patch "vpci: Hide capability when it fails to initialize".
* Whole implementation changed because last version is wrong.
This version gets target handler and previous handler from vpci->handlers, then remove the target.
* Note: a case in function vpci_ext_capability_mask() needs to be discussed,
because it may change the offset of next capability when the offset of target
capability is 0x100U(the first extended capability), my implementation is just to
ignore and let hardware to handle the target capability.
v1->v2 changes:
* Removed the "priorities" of initializing capabilities since it isn't used anymore.
* Added new function vpci_capability_mask() and vpci_ext_capability_mask() to
remove failed capability from list.
* Called vpci_make_msix_hole() in the end of init_msix().
Best regards,
Jiqian Chen.
---
xen/drivers/vpci/vpci.c | 100 +++++++++++++++++++++++++++++++++++--
xen/include/xen/pci_regs.h | 5 +-
2 files changed, 100 insertions(+), 5 deletions(-)
diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c
index 60e7654ec377..2d4794ff3dea 100644
--- a/xen/drivers/vpci/vpci.c
+++ b/xen/drivers/vpci/vpci.c
@@ -176,6 +176,98 @@ static int vpci_capability_hide(struct pci_dev *pdev, unsigned int cap)
return 0;
}
+static struct vpci_register *vpci_get_previous_ext_cap_register(
+ struct vpci *vpci, unsigned int offset)
+{
+ uint32_t header;
+ unsigned int pos = PCI_CFG_SPACE_SIZE;
+ struct vpci_register *r;
+
+ if ( offset <= PCI_CFG_SPACE_SIZE )
+ {
+ ASSERT_UNREACHABLE();
+ return NULL;
+ }
+
+ r = vpci_get_register(vpci, pos, 4);
+ if ( !r )
+ return NULL;
+
+ header = (uint32_t)(uintptr_t)r->private;
+ pos = PCI_EXT_CAP_NEXT(header);
+ while ( pos > PCI_CFG_SPACE_SIZE && pos != offset )
+ {
+ r = vpci_get_register(vpci, pos, 4);
+ if ( !r )
+ return NULL;
+ header = (uint32_t)(uintptr_t)r->private;
+ pos = PCI_EXT_CAP_NEXT(header);
+ }
+
+ if ( pos <= PCI_CFG_SPACE_SIZE )
+ return NULL;
+
+ return r;
+}
+
+static int vpci_ext_capability_hide(struct pci_dev *pdev, unsigned int cap)
+{
+ const unsigned int offset = pci_find_ext_capability(pdev->sbdf, cap);
+ struct vpci_register *rm, *prev_r;
+ struct vpci *vpci = pdev->vpci;
+ uint32_t header, pre_header;
+
+ if ( !offset )
+ {
+ ASSERT_UNREACHABLE();
+ return 0;
+ }
+
+ spin_lock(&vpci->lock);
+ rm = vpci_get_register(vpci, offset, 4);
+ if ( !rm )
+ {
+ spin_unlock(&vpci->lock);
+ return -ENODEV;
+ }
+
+ header = (uint32_t)(uintptr_t)rm->private;
+ if ( offset == PCI_CFG_SPACE_SIZE )
+ {
+ if ( PCI_EXT_CAP_NEXT(header) <= PCI_CFG_SPACE_SIZE )
+ rm->private = (void *)(uintptr_t)0;
+ else
+ /*
+ * If this case removes target capability of position 0x100U, then
+ * it needs to move the next capability to be in position 0x100U,
+ * that would cause the offset of next capability in vpci different
+ * from the hardware, then cause error accesses, so here chooses to
+ * set the capability ID part to be zero.
+ */
+ rm->private = (void *)(uintptr_t)(header &
+ ~PCI_EXT_CAP_ID(header));
+
+ spin_unlock(&vpci->lock);
+ return 0;
+ }
+
+ prev_r = vpci_get_previous_ext_cap_register(vpci, offset);
+ if ( !prev_r )
+ {
+ spin_unlock(&vpci->lock);
+ return -ENODEV;
+ }
+
+ pre_header = (uint32_t)(uintptr_t)prev_r->private;
+ prev_r->private = (void *)(uintptr_t)((pre_header &
+ ~PCI_EXT_CAP_NEXT_MASK) |
+ (header & PCI_EXT_CAP_NEXT_MASK));
+ list_del(&rm->node);
+ spin_unlock(&vpci->lock);
+ xfree(rm);
+ return 0;
+}
+
static int vpci_init_capabilities(struct pci_dev *pdev)
{
for ( unsigned int i = 0; i < NUM_VPCI_INIT; i++ )
@@ -209,11 +301,11 @@ static int vpci_init_capabilities(struct pci_dev *pdev)
pdev->domain, &pdev->sbdf,
is_ext ? "extended" : "legacy", cap);
if ( !is_ext )
- {
rc = vpci_capability_hide(pdev, cap);
- if ( rc )
- return rc;
- }
+ else
+ rc = vpci_ext_capability_hide(pdev, cap);
+ if ( rc )
+ return rc;
}
}
diff --git a/xen/include/xen/pci_regs.h b/xen/include/xen/pci_regs.h
index 27b4f44eedf3..3b6963133dbd 100644
--- a/xen/include/xen/pci_regs.h
+++ b/xen/include/xen/pci_regs.h
@@ -448,7 +448,10 @@
/* Extended Capabilities (PCI-X 2.0 and Express) */
#define PCI_EXT_CAP_ID(header) ((header) & 0x0000ffff)
#define PCI_EXT_CAP_VER(header) (((header) >> 16) & 0xf)
-#define PCI_EXT_CAP_NEXT(header) (((header) >> 20) & 0xffc)
+#define PCI_EXT_CAP_NEXT_MASK 0xfff00000U
+/* Bottom two bits of next capability position are reserved. */
+#define PCI_EXT_CAP_NEXT(header) \
+ (MASK_EXTR(header, PCI_EXT_CAP_NEXT_MASK) & 0xffcU)
#define PCI_EXT_CAP_ID_ERR 1
#define PCI_EXT_CAP_ID_VC 2
--
2.34.1
On Mon, May 26, 2025 at 05:45:55PM +0800, Jiqian Chen wrote:
> When vpci fails to initialize a extended capability of device, it
> just returns an error and vPCI gets disabled for the whole device.
>
> So, add function to hide extended capability when initialization
> fails. And remove the failed extended capability handler from vpci
> extended capability list.
>
> Signed-off-by: Jiqian Chen <Jiqian.Chen@amd.com>
> ---
> cc: "Roger Pau Monné" <roger.pau@citrix.com>
> cc: Andrew Cooper <andrew.cooper3@citrix.com>
> cc: Anthony PERARD <anthony.perard@vates.tech>
> cc: Michal Orzel <michal.orzel@amd.com>
> cc: Jan Beulich <jbeulich@suse.com>
> cc: Julien Grall <julien@xen.org>
> cc: Stefano Stabellini <sstabellini@kernel.org>
> ---
> v4->v5 changes:
> * Modify the hex digits of PCI_EXT_CAP_NEXT_MASK and PCI_EXT_CAP_NEXT to be low case.
> * Rename vpci_ext_capability_mask to vpci_ext_capability_hide.
>
> v3->v4 changes:
> * Change definition of PCI_EXT_CAP_NEXT to be "#define PCI_EXT_CAP_NEXT(header) (MASK_EXTR(header, PCI_EXT_CAP_NEXT_MASK) & 0xFFCU)" to avoid redundancy.
> * Modify the commit message.
> * Change vpci_ext_capability_mask() to return error instead of using ASSERT.
> * Set the capability ID part to be zero when we need to hide the capability of position 0x100U.
> * Add check "if ( !offset )" in vpci_ext_capability_mask().
>
> v2->v3 changes:
> * Separated from the last version patch "vpci: Hide capability when it fails to initialize".
> * Whole implementation changed because last version is wrong.
> This version gets target handler and previous handler from vpci->handlers, then remove the target.
> * Note: a case in function vpci_ext_capability_mask() needs to be discussed,
> because it may change the offset of next capability when the offset of target
> capability is 0x100U(the first extended capability), my implementation is just to
> ignore and let hardware to handle the target capability.
>
> v1->v2 changes:
> * Removed the "priorities" of initializing capabilities since it isn't used anymore.
> * Added new function vpci_capability_mask() and vpci_ext_capability_mask() to
> remove failed capability from list.
> * Called vpci_make_msix_hole() in the end of init_msix().
>
> Best regards,
> Jiqian Chen.
> ---
> xen/drivers/vpci/vpci.c | 100 +++++++++++++++++++++++++++++++++++--
> xen/include/xen/pci_regs.h | 5 +-
> 2 files changed, 100 insertions(+), 5 deletions(-)
>
> diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c
> index 60e7654ec377..2d4794ff3dea 100644
> --- a/xen/drivers/vpci/vpci.c
> +++ b/xen/drivers/vpci/vpci.c
> @@ -176,6 +176,98 @@ static int vpci_capability_hide(struct pci_dev *pdev, unsigned int cap)
> return 0;
> }
>
> +static struct vpci_register *vpci_get_previous_ext_cap_register(
> + struct vpci *vpci, unsigned int offset)
> +{
> + uint32_t header;
> + unsigned int pos = PCI_CFG_SPACE_SIZE;
> + struct vpci_register *r;
> +
> + if ( offset <= PCI_CFG_SPACE_SIZE )
> + {
> + ASSERT_UNREACHABLE();
> + return NULL;
> + }
> +
> + r = vpci_get_register(vpci, pos, 4);
> + if ( !r )
> + return NULL;
> +
> + header = (uint32_t)(uintptr_t)r->private;
> + pos = PCI_EXT_CAP_NEXT(header);
> + while ( pos > PCI_CFG_SPACE_SIZE && pos != offset )
> + {
> + r = vpci_get_register(vpci, pos, 4);
> + if ( !r )
> + return NULL;
> + header = (uint32_t)(uintptr_t)r->private;
> + pos = PCI_EXT_CAP_NEXT(header);
> + }
> +
> + if ( pos <= PCI_CFG_SPACE_SIZE )
> + return NULL;
Same comment as in the previous patch, I think the proposed for loop
there can also be used here to reduce a bit the code size (and unify
the return paths).
> +
> + return r;
> +}
> +
> +static int vpci_ext_capability_hide(struct pci_dev *pdev, unsigned int cap)
> +{
> + const unsigned int offset = pci_find_ext_capability(pdev->sbdf, cap);
> + struct vpci_register *rm, *prev_r;
s/rm/r/
> + struct vpci *vpci = pdev->vpci;
> + uint32_t header, pre_header;
> +
> + if ( !offset )
I think you want offset < PCI_CFG_SPACE_SIZE here?
> + {
> + ASSERT_UNREACHABLE();
> + return 0;
> + }
> +
> + spin_lock(&vpci->lock);
> + rm = vpci_get_register(vpci, offset, 4);
> + if ( !rm )
> + {
> + spin_unlock(&vpci->lock);
> + return -ENODEV;
> + }
> +
> + header = (uint32_t)(uintptr_t)rm->private;
> + if ( offset == PCI_CFG_SPACE_SIZE )
> + {
> + if ( PCI_EXT_CAP_NEXT(header) <= PCI_CFG_SPACE_SIZE )
> + rm->private = (void *)(uintptr_t)0;
> + else
> + /*
> + * If this case removes target capability of position 0x100U, then
> + * it needs to move the next capability to be in position 0x100U,
> + * that would cause the offset of next capability in vpci different
> + * from the hardware, then cause error accesses, so here chooses to
> + * set the capability ID part to be zero.
/*
* The first extended capability (0x100) cannot be removed from the linked
* list, so instead mask its capability ID to return 0 and force OSes
* to skip it.
*/
Is simpler IMO and conveys the same message.
> + */
> + rm->private = (void *)(uintptr_t)(header &
> + ~PCI_EXT_CAP_ID(header));
> +
> + spin_unlock(&vpci->lock);
> + return 0;
> + }
> +
> + prev_r = vpci_get_previous_ext_cap_register(vpci, offset);
> + if ( !prev_r )
> + {
> + spin_unlock(&vpci->lock);
> + return -ENODEV;
> + }
> +
> + pre_header = (uint32_t)(uintptr_t)prev_r->private;
> + prev_r->private = (void *)(uintptr_t)((pre_header &
> + ~PCI_EXT_CAP_NEXT_MASK) |
> + (header & PCI_EXT_CAP_NEXT_MASK));
No strong opinion (and your code is correct), but it might be easier
to read as:
pre_header &= ~PCI_EXT_CAP_NEXT_MASK;
pre_header |= header & PCI_EXT_CAP_NEXT_MASK;
prev_r->private = (void *)(uintptr_t)pre_header;
It's still tree lines of code at the end. I would also add a newline
to separate from the removal of rm.
> + list_del(&rm->node);
> + spin_unlock(&vpci->lock);
> + xfree(rm);
Newline before the return preferably.
> + return 0;
> +}
> +
> static int vpci_init_capabilities(struct pci_dev *pdev)
> {
> for ( unsigned int i = 0; i < NUM_VPCI_INIT; i++ )
> @@ -209,11 +301,11 @@ static int vpci_init_capabilities(struct pci_dev *pdev)
> pdev->domain, &pdev->sbdf,
> is_ext ? "extended" : "legacy", cap);
> if ( !is_ext )
> - {
> rc = vpci_capability_hide(pdev, cap);
> - if ( rc )
> - return rc;
> - }
> + else
> + rc = vpci_ext_capability_hide(pdev, cap);
> + if ( rc )
> + return rc;
Could the code in the previous patch be:
if ( !is_ext )
rc = vpci_capability_hide(pdev, cap);
if ( rc )
return rc;
So that your introduction here is simpler?
Thanks, Roger.
On 2025/6/5 22:47, Roger Pau Monné wrote:
> On Mon, May 26, 2025 at 05:45:55PM +0800, Jiqian Chen wrote:
>> When vpci fails to initialize a extended capability of device, it
>> just returns an error and vPCI gets disabled for the whole device.
>>
>> So, add function to hide extended capability when initialization
>> fails. And remove the failed extended capability handler from vpci
>> extended capability list.
>>
>> Signed-off-by: Jiqian Chen <Jiqian.Chen@amd.com>
>> ---
>> cc: "Roger Pau Monné" <roger.pau@citrix.com>
>> cc: Andrew Cooper <andrew.cooper3@citrix.com>
>> cc: Anthony PERARD <anthony.perard@vates.tech>
>> cc: Michal Orzel <michal.orzel@amd.com>
>> cc: Jan Beulich <jbeulich@suse.com>
>> cc: Julien Grall <julien@xen.org>
>> cc: Stefano Stabellini <sstabellini@kernel.org>
>> ---
>> v4->v5 changes:
>> * Modify the hex digits of PCI_EXT_CAP_NEXT_MASK and PCI_EXT_CAP_NEXT to be low case.
>> * Rename vpci_ext_capability_mask to vpci_ext_capability_hide.
>>
>> v3->v4 changes:
>> * Change definition of PCI_EXT_CAP_NEXT to be "#define PCI_EXT_CAP_NEXT(header) (MASK_EXTR(header, PCI_EXT_CAP_NEXT_MASK) & 0xFFCU)" to avoid redundancy.
>> * Modify the commit message.
>> * Change vpci_ext_capability_mask() to return error instead of using ASSERT.
>> * Set the capability ID part to be zero when we need to hide the capability of position 0x100U.
>> * Add check "if ( !offset )" in vpci_ext_capability_mask().
>>
>> v2->v3 changes:
>> * Separated from the last version patch "vpci: Hide capability when it fails to initialize".
>> * Whole implementation changed because last version is wrong.
>> This version gets target handler and previous handler from vpci->handlers, then remove the target.
>> * Note: a case in function vpci_ext_capability_mask() needs to be discussed,
>> because it may change the offset of next capability when the offset of target
>> capability is 0x100U(the first extended capability), my implementation is just to
>> ignore and let hardware to handle the target capability.
>>
>> v1->v2 changes:
>> * Removed the "priorities" of initializing capabilities since it isn't used anymore.
>> * Added new function vpci_capability_mask() and vpci_ext_capability_mask() to
>> remove failed capability from list.
>> * Called vpci_make_msix_hole() in the end of init_msix().
>>
>> Best regards,
>> Jiqian Chen.
>> ---
>> xen/drivers/vpci/vpci.c | 100 +++++++++++++++++++++++++++++++++++--
>> xen/include/xen/pci_regs.h | 5 +-
>> 2 files changed, 100 insertions(+), 5 deletions(-)
>>
>> diff --git a/xen/drivers/vpci/vpci.c b/xen/drivers/vpci/vpci.c
>> index 60e7654ec377..2d4794ff3dea 100644
>> --- a/xen/drivers/vpci/vpci.c
>> +++ b/xen/drivers/vpci/vpci.c
>> @@ -176,6 +176,98 @@ static int vpci_capability_hide(struct pci_dev *pdev, unsigned int cap)
>> return 0;
>> }
>>
>> +static struct vpci_register *vpci_get_previous_ext_cap_register(
>> + struct vpci *vpci, unsigned int offset)
>> +{
>> + uint32_t header;
>> + unsigned int pos = PCI_CFG_SPACE_SIZE;
>> + struct vpci_register *r;
>> +
>> + if ( offset <= PCI_CFG_SPACE_SIZE )
>> + {
>> + ASSERT_UNREACHABLE();
>> + return NULL;
>> + }
>> +
>> + r = vpci_get_register(vpci, pos, 4);
>> + if ( !r )
>> + return NULL;
>> +
>> + header = (uint32_t)(uintptr_t)r->private;
>> + pos = PCI_EXT_CAP_NEXT(header);
>> + while ( pos > PCI_CFG_SPACE_SIZE && pos != offset )
>> + {
>> + r = vpci_get_register(vpci, pos, 4);
>> + if ( !r )
>> + return NULL;
>> + header = (uint32_t)(uintptr_t)r->private;
>> + pos = PCI_EXT_CAP_NEXT(header);
>> + }
>> +
>> + if ( pos <= PCI_CFG_SPACE_SIZE )
>> + return NULL;
>
> Same comment as in the previous patch, I think the proposed for loop
> there can also be used here to reduce a bit the code size (and unify
> the return paths).
>
>> +
>> + return r;
>> +}
>> +
>> +static int vpci_ext_capability_hide(struct pci_dev *pdev, unsigned int cap)
>> +{
>> + const unsigned int offset = pci_find_ext_capability(pdev->sbdf, cap);
>> + struct vpci_register *rm, *prev_r;
>
> s/rm/r/
>
>> + struct vpci *vpci = pdev->vpci;
>> + uint32_t header, pre_header;
>> +
>> + if ( !offset )
>
> I think you want offset < PCI_CFG_SPACE_SIZE here?
>
>> + {
>> + ASSERT_UNREACHABLE();
>> + return 0;
>> + }
>> +
>> + spin_lock(&vpci->lock);
>> + rm = vpci_get_register(vpci, offset, 4);
>> + if ( !rm )
>> + {
>> + spin_unlock(&vpci->lock);
>> + return -ENODEV;
>> + }
>> +
>> + header = (uint32_t)(uintptr_t)rm->private;
>> + if ( offset == PCI_CFG_SPACE_SIZE )
>> + {
>> + if ( PCI_EXT_CAP_NEXT(header) <= PCI_CFG_SPACE_SIZE )
>> + rm->private = (void *)(uintptr_t)0;
>> + else
>> + /*
>> + * If this case removes target capability of position 0x100U, then
>> + * it needs to move the next capability to be in position 0x100U,
>> + * that would cause the offset of next capability in vpci different
>> + * from the hardware, then cause error accesses, so here chooses to
>> + * set the capability ID part to be zero.
>
> /*
> * The first extended capability (0x100) cannot be removed from the linked
> * list, so instead mask its capability ID to return 0 and force OSes
> * to skip it.
> */
>
> Is simpler IMO and conveys the same message.
>
>> + */
>> + rm->private = (void *)(uintptr_t)(header &
>> + ~PCI_EXT_CAP_ID(header));
>> +
>> + spin_unlock(&vpci->lock);
>> + return 0;
>> + }
>> +
>> + prev_r = vpci_get_previous_ext_cap_register(vpci, offset);
>> + if ( !prev_r )
>> + {
>> + spin_unlock(&vpci->lock);
>> + return -ENODEV;
>> + }
>> +
>> + pre_header = (uint32_t)(uintptr_t)prev_r->private;
>> + prev_r->private = (void *)(uintptr_t)((pre_header &
>> + ~PCI_EXT_CAP_NEXT_MASK) |
>> + (header & PCI_EXT_CAP_NEXT_MASK));
>
> No strong opinion (and your code is correct), but it might be easier
> to read as:
>
> pre_header &= ~PCI_EXT_CAP_NEXT_MASK;
> pre_header |= header & PCI_EXT_CAP_NEXT_MASK;
> prev_r->private = (void *)(uintptr_t)pre_header;
>
> It's still tree lines of code at the end. I would also add a newline
> to separate from the removal of rm.
>
>> + list_del(&rm->node);
>> + spin_unlock(&vpci->lock);
>> + xfree(rm);
>
> Newline before the return preferably.
Will change my patch according to all your comments.
>
>> + return 0;
>> +}
>> +
>> static int vpci_init_capabilities(struct pci_dev *pdev)
>> {
>> for ( unsigned int i = 0; i < NUM_VPCI_INIT; i++ )
>> @@ -209,11 +301,11 @@ static int vpci_init_capabilities(struct pci_dev *pdev)
>> pdev->domain, &pdev->sbdf,
>> is_ext ? "extended" : "legacy", cap);
>> if ( !is_ext )
>> - {
>> rc = vpci_capability_hide(pdev, cap);
>> - if ( rc )
>> - return rc;
>> - }
>> + else
>> + rc = vpci_ext_capability_hide(pdev, cap);
>> + if ( rc )
>> + return rc;
>
> Could the code in the previous patch be:
>
> if ( !is_ext )
> rc = vpci_capability_hide(pdev, cap);
>
> if ( rc )
> return rc;
>
> So that your introduction here is simpler?
OK, but the logic of the previous patch will become a little strange.
Anyway, the strange will disappear after applying this patch.
>
> Thanks, Roger.
--
Best regards,
Jiqian Chen.
On Fri, Jun 06, 2025 at 08:30:42AM +0000, Chen, Jiqian wrote:
> On 2025/6/5 22:47, Roger Pau Monné wrote:
> > On Mon, May 26, 2025 at 05:45:55PM +0800, Jiqian Chen wrote:
> >> @@ -209,11 +301,11 @@ static int vpci_init_capabilities(struct pci_dev *pdev)
> >> pdev->domain, &pdev->sbdf,
> >> is_ext ? "extended" : "legacy", cap);
> >> if ( !is_ext )
> >> - {
> >> rc = vpci_capability_hide(pdev, cap);
> >> - if ( rc )
> >> - return rc;
> >> - }
> >> + else
> >> + rc = vpci_ext_capability_hide(pdev, cap);
> >> + if ( rc )
> >> + return rc;
> >
> > Could the code in the previous patch be:
> >
> > if ( !is_ext )
> > rc = vpci_capability_hide(pdev, cap);
> >
> > if ( rc )
> > return rc;
> >
> > So that your introduction here is simpler?
> OK, but the logic of the previous patch will become a little strange.
> Anyway, the strange will disappear after applying this patch.
No strong opinion really, was mostly a recommendation to avoid extra
changes here. In a series it's best if you try to arrange the code so
that it's only modified once (if possible, obviously).
Thanks, Roger.
© 2016 - 2025 Red Hat, Inc.