Mailing List Archive

[PATCH V2 08/23] xen/ioreq: Introduce ioreq_params to abstract accesses to arch.hvm.params
From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>

We don't want to move HVM params field out of *arch.hvm* in this particular
case as although it stores a few IOREQ params, it is not a (completely)
IOREQ stuff and is specific to the architecture. Instead, abstract
accesses by the proposed macro.

This is a follow up action to reduce layering violation in the common code.

Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
CC: Julien Grall <julien.grall@arm.com>

---
Please note, this is a split/cleanup/hardening of Julien's PoC:
"Add support for Guest IO forwarding to a device emulator"

Changes V1 -> V2:
- new patch
---
xen/common/ioreq.c | 4 ++--
xen/include/asm-x86/hvm/domain.h | 2 ++
2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/xen/common/ioreq.c b/xen/common/ioreq.c
index 7f91bc2..a07f1d7 100644
--- a/xen/common/ioreq.c
+++ b/xen/common/ioreq.c
@@ -223,7 +223,7 @@ static gfn_t hvm_alloc_legacy_ioreq_gfn(struct ioreq_server *s)
for ( i = HVM_PARAM_IOREQ_PFN; i <= HVM_PARAM_BUFIOREQ_PFN; i++ )
{
if ( !test_and_clear_bit(i, &d->ioreq_gfn.legacy_mask) )
- return _gfn(d->arch.hvm.params[i]);
+ return _gfn(ioreq_params(d, i));
}

return INVALID_GFN;
@@ -255,7 +255,7 @@ static bool hvm_free_legacy_ioreq_gfn(struct ioreq_server *s,

for ( i = HVM_PARAM_IOREQ_PFN; i <= HVM_PARAM_BUFIOREQ_PFN; i++ )
{
- if ( gfn_eq(gfn, _gfn(d->arch.hvm.params[i])) )
+ if ( gfn_eq(gfn, _gfn(ioreq_params(d, i))) )
break;
}
if ( i > HVM_PARAM_BUFIOREQ_PFN )
diff --git a/xen/include/asm-x86/hvm/domain.h b/xen/include/asm-x86/hvm/domain.h
index 5d60737..c3af339 100644
--- a/xen/include/asm-x86/hvm/domain.h
+++ b/xen/include/asm-x86/hvm/domain.h
@@ -63,6 +63,8 @@ struct hvm_pi_ops {
void (*vcpu_block)(struct vcpu *);
};

+#define ioreq_params(d, i) ((d)->arch.hvm.params[i])
+
struct hvm_domain {
/* Cached CF8 for guest PCI config cycles */
uint32_t pci_cf8;
--
2.7.4
RE: [PATCH V2 08/23] xen/ioreq: Introduce ioreq_params to abstract accesses to arch.hvm.params [ In reply to ]
> -----Original Message-----
> From: Oleksandr Tyshchenko <olekstysh@gmail.com>
> Sent: 15 October 2020 17:44
> To: xen-devel@lists.xenproject.org
> Cc: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>; Paul Durrant <paul@xen.org>; Jan Beulich
> <jbeulich@suse.com>; Andrew Cooper <andrew.cooper3@citrix.com>; Roger Pau Monné
> <roger.pau@citrix.com>; Wei Liu <wl@xen.org>; Julien Grall <julien@xen.org>; Stefano Stabellini
> <sstabellini@kernel.org>; Julien Grall <julien.grall@arm.com>
> Subject: [PATCH V2 08/23] xen/ioreq: Introduce ioreq_params to abstract accesses to arch.hvm.params
>
> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>
> We don't want to move HVM params field out of *arch.hvm* in this particular
> case as although it stores a few IOREQ params, it is not a (completely)
> IOREQ stuff and is specific to the architecture. Instead, abstract
> accesses by the proposed macro.
>
> This is a follow up action to reduce layering violation in the common code.
>
> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> CC: Julien Grall <julien.grall@arm.com>
>

Keeping the 'legacy' magic page code under an x86 ioreq.c would avoid the need for this patch.

Paul

> ---
> Please note, this is a split/cleanup/hardening of Julien's PoC:
> "Add support for Guest IO forwarding to a device emulator"
>
> Changes V1 -> V2:
> - new patch
> ---
> xen/common/ioreq.c | 4 ++--
> xen/include/asm-x86/hvm/domain.h | 2 ++
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/xen/common/ioreq.c b/xen/common/ioreq.c
> index 7f91bc2..a07f1d7 100644
> --- a/xen/common/ioreq.c
> +++ b/xen/common/ioreq.c
> @@ -223,7 +223,7 @@ static gfn_t hvm_alloc_legacy_ioreq_gfn(struct ioreq_server *s)
> for ( i = HVM_PARAM_IOREQ_PFN; i <= HVM_PARAM_BUFIOREQ_PFN; i++ )
> {
> if ( !test_and_clear_bit(i, &d->ioreq_gfn.legacy_mask) )
> - return _gfn(d->arch.hvm.params[i]);
> + return _gfn(ioreq_params(d, i));
> }
>
> return INVALID_GFN;
> @@ -255,7 +255,7 @@ static bool hvm_free_legacy_ioreq_gfn(struct ioreq_server *s,
>
> for ( i = HVM_PARAM_IOREQ_PFN; i <= HVM_PARAM_BUFIOREQ_PFN; i++ )
> {
> - if ( gfn_eq(gfn, _gfn(d->arch.hvm.params[i])) )
> + if ( gfn_eq(gfn, _gfn(ioreq_params(d, i))) )
> break;
> }
> if ( i > HVM_PARAM_BUFIOREQ_PFN )
> diff --git a/xen/include/asm-x86/hvm/domain.h b/xen/include/asm-x86/hvm/domain.h
> index 5d60737..c3af339 100644
> --- a/xen/include/asm-x86/hvm/domain.h
> +++ b/xen/include/asm-x86/hvm/domain.h
> @@ -63,6 +63,8 @@ struct hvm_pi_ops {
> void (*vcpu_block)(struct vcpu *);
> };
>
> +#define ioreq_params(d, i) ((d)->arch.hvm.params[i])
> +
> struct hvm_domain {
> /* Cached CF8 for guest PCI config cycles */
> uint32_t pci_cf8;
> --
> 2.7.4
Re: [PATCH V2 08/23] xen/ioreq: Introduce ioreq_params to abstract accesses to arch.hvm.params [ In reply to ]
On 20.10.20 13:41, Paul Durrant wrote:

Hi Paul

Sorry for the late response.


>> -----Original Message-----
>> From: Oleksandr Tyshchenko <olekstysh@gmail.com>
>> Sent: 15 October 2020 17:44
>> To: xen-devel@lists.xenproject.org
>> Cc: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>; Paul Durrant <paul@xen.org>; Jan Beulich
>> <jbeulich@suse.com>; Andrew Cooper <andrew.cooper3@citrix.com>; Roger Pau Monné
>> <roger.pau@citrix.com>; Wei Liu <wl@xen.org>; Julien Grall <julien@xen.org>; Stefano Stabellini
>> <sstabellini@kernel.org>; Julien Grall <julien.grall@arm.com>
>> Subject: [PATCH V2 08/23] xen/ioreq: Introduce ioreq_params to abstract accesses to arch.hvm.params
>>
>> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>>
>> We don't want to move HVM params field out of *arch.hvm* in this particular
>> case as although it stores a few IOREQ params, it is not a (completely)
>> IOREQ stuff and is specific to the architecture. Instead, abstract
>> accesses by the proposed macro.
>>
>> This is a follow up action to reduce layering violation in the common code.
>>
>> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>> CC: Julien Grall <julien.grall@arm.com>
>>
> Keeping the 'legacy' magic page code under an x86 ioreq.c would avoid the need for this patch.

In that case, yes, agree.


--
Regards,

Oleksandr Tyshchenko