Re: [RFC XEN PATCH 06/16] tools: reserve guest memory for ACPI from device model

[Konrad Rzeszutek Wilk <konrad@darnok.org>]

On Mon, Oct 10, 2016 at 08:32:25AM +0800, Haozhong Zhang wrote:
> One guest page is reserved for the device model to place guest ACPI. The

guest ACPI what? ACPI SSDT? MADT?

Also why one page? What if there is a need for more than one page?

You add  HVM_XS_DM_ACPI_LENGTH which makes me think this is accounted
for?

> base address and size of the reserved area are passed to the device
> model via XenStore keys hvmloader/dm-acpi/{address, length}.
> 
> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
> ---
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> ---
>  tools/libxc/include/xc_dom.h            |  1 +
>  tools/libxc/xc_dom_x86.c                |  7 +++++++
>  tools/libxl/libxl_dom.c                 | 25 +++++++++++++++++++++++++
>  xen/include/public/hvm/hvm_xs_strings.h | 11 +++++++++++
>  4 files changed, 44 insertions(+)
> 
> diff --git a/tools/libxc/include/xc_dom.h b/tools/libxc/include/xc_dom.h
> index 608cbc2..19d65cd 100644
> --- a/tools/libxc/include/xc_dom.h
> +++ b/tools/libxc/include/xc_dom.h
> @@ -98,6 +98,7 @@ struct xc_dom_image {
>      xen_pfn_t xenstore_pfn;
>      xen_pfn_t shared_info_pfn;
>      xen_pfn_t bootstack_pfn;
> +    xen_pfn_t dm_acpi_pfn;

Perhaps an pointer to an variable size array?

 xen_pfn_t *dm_acpi_pfns;
 unsigned int dm_apci_nr;

?
>      xen_pfn_t pfn_alloc_end;
>      xen_vaddr_t virt_alloc_end;
>      xen_vaddr_t bsd_symtab_start;
> diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c
> index 0eab8a7..47f14a1 100644
> --- a/tools/libxc/xc_dom_x86.c
> +++ b/tools/libxc/xc_dom_x86.c
> @@ -674,6 +674,13 @@ static int alloc_magic_pages_hvm(struct xc_dom_image *dom)
>                           ioreq_server_pfn(0));
>          xc_hvm_param_set(xch, domid, HVM_PARAM_NR_IOREQ_SERVER_PAGES,
>                           NR_IOREQ_SERVER_PAGES);
> +
> +        dom->dm_acpi_pfn = xc_dom_alloc_page(dom, "DM ACPI");
> +        if ( dom->dm_acpi_pfn == INVALID_PFN )
> +        {
> +            DOMPRINTF("Could not allocate page for device model ACPI.");
> +            goto error_out;
> +        }
>      }
>  
>      rc = xc_dom_alloc_segment(dom, &dom->start_info_seg,
> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
> index d519c8d..f0a1d97 100644
> --- a/tools/libxl/libxl_dom.c
> +++ b/tools/libxl/libxl_dom.c
> @@ -865,6 +865,31 @@ static int hvm_build_set_xs_values(libxl__gc *gc,
>              goto err;
>      }
>  
> +    if (dom->dm_acpi_pfn) {
> +        uint64_t guest_addr_out = dom->dm_acpi_pfn * XC_DOM_PAGE_SIZE(dom);
> +
> +        if (guest_addr_out >= 0x100000000ULL) {
> +            LOG(ERROR,
> +                "Guest address of DM ACPI is 0x%"PRIx64", but expected below 4G",
> +                guest_addr_out);
> +            goto err;
> +        }
> +
> +        path = GCSPRINTF("/local/domain/%d/"HVM_XS_DM_ACPI_ADDRESS, domid);
> +
> +        ret = libxl__xs_printf(gc, XBT_NULL, path, "0x%"PRIx64,
> +                               guest_addr_out);
> +        if (ret)
> +            goto err;
> +
> +        path = GCSPRINTF("/local/domain/%d/"HVM_XS_DM_ACPI_LENGTH, domid);
> +
> +        ret = libxl__xs_printf(gc, XBT_NULL, path, "0x%"PRIx64,
> +                               (uint64_t) XC_DOM_PAGE_SIZE(dom));
I don't think you need the space here:      ^
> +        if (ret)
> +            goto err;
> +    }
> +
>      return 0;
>  
>  err:
> diff --git a/xen/include/public/hvm/hvm_xs_strings.h b/xen/include/public/hvm/hvm_xs_strings.h
> index 146b0b0..f44f71f 100644
> --- a/xen/include/public/hvm/hvm_xs_strings.h
> +++ b/xen/include/public/hvm/hvm_xs_strings.h
> @@ -79,4 +79,15 @@
>   */
>  #define HVM_XS_OEM_STRINGS             "bios-strings/oem-%d"
>  
> +/* Follows are XenStore keys for DM ACPI (ACPI built by device model,
> + * e.g. QEMU).
> + *
> + * A reserved area of guest physical memory is used to pass DM
> + * ACPI. Values of following two keys specify the base address and
> + * length (in bytes) of the reserved area.
> + */
> +#define HVM_XS_DM_ACPI_ROOT              "hvmloader/dm-acpi"
> +#define HVM_XS_DM_ACPI_ADDRESS           HVM_XS_DM_ACPI_ROOT"/address"
> +#define HVM_XS_DM_ACPI_LENGTH            HVM_XS_DM_ACPI_ROOT"/length"
> +
>  #endif /* __XEN_PUBLIC_HVM_HVM_XS_STRINGS_H__ */
> -- 
> 2.10.1
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel

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