[Xen-devel] [PATCH v3 01/10] ioreq: terminate cf8 handling at hypervisor level

Roger Pau Monne posted 10 patches 6 years, 4 months ago
[Xen-devel] [PATCH v3 01/10] ioreq: terminate cf8 handling at hypervisor level
Posted by Roger Pau Monne 6 years, 4 months ago
Do not forward accesses to cf8 to external emulators, decoding of PCI
accesses is handled by Xen, and emulators can request handling of
config space accesses of devices using the provided ioreq interface.

Fully terminate cf8 accesses at the hypervisor level, by improving the
existing hvm_access_cf8 helper to also handle register reads, and
always return X86EMUL_OKAY in order to terminate the emulation.

Note that without this change in the absence of some external emulator
that catches accesses to cf8 read requests to the register would
misbehave, as the ioreq internal handler did not handle those.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Changes since v2:
 - Allow ioreq servers to map 0xcf8 and 0xcfc, even if those are
   handled by the hypervisor.

Changes since v1:
 - New in this version.
---
 xen/arch/x86/hvm/ioreq.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/hvm/ioreq.c b/xen/arch/x86/hvm/ioreq.c
index d347144096..5e503ce498 100644
--- a/xen/arch/x86/hvm/ioreq.c
+++ b/xen/arch/x86/hvm/ioreq.c
@@ -1518,11 +1518,15 @@ static int hvm_access_cf8(
 {
     struct domain *d = current->domain;
 
-    if ( dir == IOREQ_WRITE && bytes == 4 )
+    if ( bytes != 4 )
+        return X86EMUL_OKAY;
+
+    if ( dir == IOREQ_WRITE )
         d->arch.hvm.pci_cf8 = *val;
+    else
+        *val = d->arch.hvm.pci_cf8;
 
-    /* We always need to fall through to the catch all emulator */
-    return X86EMUL_UNHANDLEABLE;
+    return X86EMUL_OKAY;
 }
 
 void hvm_ioreq_init(struct domain *d)
-- 
2.23.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 01/10] ioreq: terminate cf8 handling at hypervisor level
Posted by Paul Durrant 6 years, 4 months ago
> -----Original Message-----
> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of Roger Pau Monne
> Sent: 30 September 2019 14:32
> To: xen-devel@lists.xenproject.org
> Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Paul Durrant <paul@xen.org>; Wei Liu <wl@xen.org>; Jan
> Beulich <jbeulich@suse.com>; Roger Pau Monne <roger.pau@citrix.com>
> Subject: [Xen-devel] [PATCH v3 01/10] ioreq: terminate cf8 handling at hypervisor level
> 
> Do not forward accesses to cf8 to external emulators, decoding of PCI
> accesses is handled by Xen, and emulators can request handling of
> config space accesses of devices using the provided ioreq interface.
> 
> Fully terminate cf8 accesses at the hypervisor level, by improving the
> existing hvm_access_cf8 helper to also handle register reads, and
> always return X86EMUL_OKAY in order to terminate the emulation.
> 
> Note that without this change in the absence of some external emulator
> that catches accesses to cf8 read requests to the register would
> misbehave, as the ioreq internal handler did not handle those.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Reviewed-by: Paul Durrant <paul@xen.org>

> ---
> Changes since v2:
>  - Allow ioreq servers to map 0xcf8 and 0xcfc, even if those are
>    handled by the hypervisor.
> 
> Changes since v1:
>  - New in this version.
> ---
>  xen/arch/x86/hvm/ioreq.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/ioreq.c b/xen/arch/x86/hvm/ioreq.c
> index d347144096..5e503ce498 100644
> --- a/xen/arch/x86/hvm/ioreq.c
> +++ b/xen/arch/x86/hvm/ioreq.c
> @@ -1518,11 +1518,15 @@ static int hvm_access_cf8(
>  {
>      struct domain *d = current->domain;
> 
> -    if ( dir == IOREQ_WRITE && bytes == 4 )
> +    if ( bytes != 4 )
> +        return X86EMUL_OKAY;
> +
> +    if ( dir == IOREQ_WRITE )
>          d->arch.hvm.pci_cf8 = *val;
> +    else
> +        *val = d->arch.hvm.pci_cf8;
> 
> -    /* We always need to fall through to the catch all emulator */
> -    return X86EMUL_UNHANDLEABLE;
> +    return X86EMUL_OKAY;
>  }
> 
>  void hvm_ioreq_init(struct domain *d)
> --
> 2.23.0
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH v3 01/10] ioreq: terminate cf8 handling at hypervisor level
Posted by Jan Beulich 6 years, 4 months ago
On 30.09.2019 15:32, Roger Pau Monne wrote:
> --- a/xen/arch/x86/hvm/ioreq.c
> +++ b/xen/arch/x86/hvm/ioreq.c
> @@ -1518,11 +1518,15 @@ static int hvm_access_cf8(
>  {
>      struct domain *d = current->domain;
>  
> -    if ( dir == IOREQ_WRITE && bytes == 4 )
> +    if ( bytes != 4 )
> +        return X86EMUL_OKAY;

I think it was already on v1 that Andrew had pointed out that e.g.
a 1-bye access to CF9 should still be forwarded. I guess you mean
to use X86EMUL_UNHANDLEABLE here, just like was done ...

> +    if ( dir == IOREQ_WRITE )
>          d->arch.hvm.pci_cf8 = *val;
> +    else
> +        *val = d->arch.hvm.pci_cf8;
>  
> -    /* We always need to fall through to the catch all emulator */
> -    return X86EMUL_UNHANDLEABLE;
> +    return X86EMUL_OKAY;
>  }

... universally before. The comment (suitably adjusted) may then
also want to move up.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel