Re: [Xen-devel] [PATCH v4 06/15] domctl: Add XEN_DOMCTL_acpi_access

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 29.11.16 at 16:33, <boris.ostrovsky@xxxxxxxxxx> wrote:
> --- /dev/null
> +++ b/xen/arch/x86/hvm/acpi.c
> @@ -0,0 +1,24 @@
> +/* acpi.c: ACPI access handling
> + *
> + * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
> + */
> +#include <xen/errno.h>
> +#include <xen/lib.h>
> +#include <xen/sched.h>
> +
> +
> +int hvm_acpi_domctl_access(struct domain *d, uint8_t rw, gas_t *gas,

I guess the gas pointer can be const?

> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -1144,6 +1144,29 @@ struct xen_domctl_psr_cat_op {
>  typedef struct xen_domctl_psr_cat_op xen_domctl_psr_cat_op_t;
>  DEFINE_XEN_GUEST_HANDLE(xen_domctl_psr_cat_op_t);
>  
> +/* ACPI Generic Address Structure */
> +typedef struct gas {

xen_acpi_gas

> +#define XEN_ACPI_SYSTEM_MEMORY 0
> +#define XEN_ACPI_SYSTEM_IO     1
> +    uint8_t    space_id;           /* Address space */
> +    uint8_t    bit_width;          /* Size in bits of given register */
> +    uint8_t    bit_offset;         /* Bit offset within the register */
> +    uint8_t    access_width;       /* Minimum Access size (ACPI 3.0) */
> +    uint64_t   address;            /* 64-bit address of register */

uint64_aligned_t with explicit padding added ahead of it.

And then there's the question of what uses of this will look like:
I'm not convinced we need to stick to the exact ACPI layout
here, unless you expect (or could imagine) for the tool stack to
hold GAS structures coming from elsewhere in its hands. If we
don't follow the layout as strictly, we could namely widen
bit_width (and maybe bit_offset) to allow for larger transfers
in one go. And in such a relaxed model I don't think we'd need
access_width at all as a field.

> +} gas_t;

xen_acpi_gas_t

> +
> +struct xen_domctl_acpi_access {
> +    gas_t      gas;                    /* IN: Register being accessed */
> +
> +#define XEN_DOMCTL_ACPI_READ   0
> +#define XEN_DOMCTL_ACPI_WRITE  1
> +    uint8_t    rw;                     /* IN: Read or write */
> +
> +    XEN_GUEST_HANDLE_64(uint8) val;    /* IN/OUT: data */

I'd prefer if we made this void, as there's no type associated with
the data really. And please add explicit padding again.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel