On 08.12.21 14:43, Andrew Cooper wrote:
On 08/12/2021 08:47, Juergen Gross wrote:The HVM parameters for pre-allocated event channels should be set in libxenguest, like it is done for PV guests and for the pre-allocated ring pages.Suggested-by: Andrew Cooper <[email protected]> Signed-off-by: Juergen Gross <[email protected]>I'm not sure that we have the concept of pre-allocated ring pages. For PV, we have: dom->console_pfn = xc_dom_alloc_page(dom, "console"); if ( dom->console_pfn == INVALID_PFN ) return -1; xc_clear_domain_page(dom->xch, dom->guest_domid, xc_dom_p2m(dom, dom->console_pfn)); and for HVM, we have: dom->console_pfn = special_pfn(SPECIALPAGE_CONSOLE); xc_clear_domain_page(dom->xch, dom->guest_domid, dom->console_pfn);
Isn't that a pre-allocation? The PFNs are fixed at boot time of the guest.
With a suitable tweak to the commit message (probably just deleting the final clause), Reivewed-by: Andrew Cooper <[email protected]> That said...diff --git a/tools/libs/light/libxl_dom.c b/tools/libs/light/libxl_dom.c index fe9f760f71..c9c24666cd 100644 --- a/tools/libs/light/libxl_dom.c +++ b/tools/libs/light/libxl_dom.c @@ -723,9 +723,8 @@ out:static int hvm_build_set_params(xc_interface *handle, uint32_t domid,libxl_domain_build_info *info, - int store_evtchn, unsigned long *store_mfn, - int console_evtchn, unsigned long *console_mfn, - domid_t store_domid, domid_t console_domid) + unsigned long *store_mfn, + unsigned long *console_mfn) { struct hvm_info_table *va_hvm; uint8_t *va_map, sum; @@ -752,8 +751,6 @@ static int hvm_build_set_params(xc_interface *handle, uint32_t domid,xc_hvm_param_get(handle, domid, HVM_PARAM_STORE_PFN, &str_mfn);xc_hvm_param_get(handle, domid, HVM_PARAM_CONSOLE_PFN, &cons_mfn);... these are junk too. I'm dismayed at how much of the toolstack tries passing function parameters via HVM_PARAMS. libxl's HVM path ought to mirror the PV path and, after libxl__build_dom() is called, just read the values back out: state->console_mfn = dom->console_pfn; state->store_mfn = dom->xenstore_pfn; That then leaves hvm_build_set_params() doing nothing but adjusting the HVM info table for real HVM guests. dom->max_vcpus is already present which covers 2 of the 3 fields, leaving only the ACPI boolean left to pass. So by passing the ACPI boolean down, we get rid of hvm_build_set_params() entirely, which seems like a very good move.
Yes, this should be in another patch, though. Juergen
OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key
OpenPGP_signature
Description: OpenPGP digital signature
