Re: [Xen-devel] [PATCH v5 03/13] domctl: Add XEN_DOMCTL_acpi_access

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

On 12/19/2016 09:17 AM, Jan Beulich wrote:
>>>> On 17.12.16 at 00:18, <boris.ostrovsky@xxxxxxxxxx> wrote:
>> --- a/xen/arch/x86/domctl.c
>> +++ b/xen/arch/x86/domctl.c
>> @@ -1425,6 +1425,15 @@ long arch_do_domctl(
>>          }
>>          break;
>>  
>> +    case XEN_DOMCTL_acpi_access:
>> +        if ( !is_hvm_domain(d) )
>> +            ret = -EINVAL;
> I think it would be better to use some other, less frequently used
> error code here.

ENODEV? (Because there is no ACPI "device" for PV guests)

>
>> --- /dev/null
>> +++ b/xen/arch/x86/hvm/acpi.c
>> @@ -0,0 +1,25 @@
>> +/* 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,
> Wouldn't "rw" better be bool internally?

I will drop it altogether since as you pointed out below there is no
reason to keep xen_acpi_access_t as a separate structure. I'll pass
whole xen_domctl_acpi_access_t here.


(And I noticed that XEN_DOMCTL_ACPI_WRITE crept into acpi_common_*()
code in patch 5 so I will want to fix dir/rw there)

-boris

>
>> --- 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 access structure */
>> +typedef struct xen_acpi_access {
>> +#define XEN_ACPI_SYSTEM_MEMORY 0
>> +#define XEN_ACPI_SYSTEM_IO     1
>> +    uint8_t            space_id;           /* Address space */
>> +    uint8_t            width;              /* Access size (bytes) */
>> +    uint8_t            pad[6];
>> +    uint64_aligned_t   address;            /* 64-bit address of register */
>> +} xen_acpi_access_t;
>> +
>> +struct xen_domctl_acpi_access {
>> +    xen_acpi_access_t  access;             /* IN: Register being accessed */
>> +
>> +#define XEN_DOMCTL_ACPI_READ   0
>> +#define XEN_DOMCTL_ACPI_WRITE  1
>> +    uint8_t    rw;                         /* IN: Read or write */
>> +    uint8_t    pad[7];
>> +
>> +    XEN_GUEST_HANDLE_64(void) val;         /* IN/OUT: data */
>> +};
>> +typedef struct xen_domctl_acpi_access xen_domctl_acpi_access_t;
>> +DEFINE_XEN_GUEST_HANDLE(xen_domctl_acpi_access_t);
> Do you expect to use xen_acpi_access anywhere else? If not,
> the overall amount of padding needed could be reduced by
> folding both structures.



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