* [PATCH v3 0/2] tools/helpers: PVH xenstore-stubdom console fixes
@ 2021-12-08 8:47 Juergen Gross
2021-12-08 8:47 ` [PATCH v3 1/2] tools/helpers: fix PVH xenstore-stubdom console parameters Juergen Gross
2021-12-08 8:47 ` [PATCH v3 2/2] tools: set event channel HVM parameters in libxenguest Juergen Gross
0 siblings, 2 replies; 9+ messages in thread
From: Juergen Gross @ 2021-12-08 8:47 UTC (permalink / raw)
To: xen-devel; +Cc: Juergen Gross, Ian Jackson, Wei Liu, Anthony PERARD
The console parameters for a PVH Xenstore-stubdom have been missing
or were just wrong.
Juergen Gross (2):
tools/helpers: fix PVH xenstore-stubdom console parameters
tools: set event channel HVM parameters in libxenguest
tools/helpers/init-xenstore-domain.c | 8 +++++---
tools/libs/guest/xg_dom_x86.c | 6 ++++++
tools/libs/light/libxl_dom.c | 15 ++++++---------
3 files changed, 17 insertions(+), 12 deletions(-)
--
2.26.2
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3 1/2] tools/helpers: fix PVH xenstore-stubdom console parameters
2021-12-08 8:47 [PATCH v3 0/2] tools/helpers: PVH xenstore-stubdom console fixes Juergen Gross
@ 2021-12-08 8:47 ` Juergen Gross
2021-12-08 8:47 ` [PATCH v3 2/2] tools: set event channel HVM parameters in libxenguest Juergen Gross
1 sibling, 0 replies; 9+ messages in thread
From: Juergen Gross @ 2021-12-08 8:47 UTC (permalink / raw)
To: xen-devel; +Cc: Juergen Gross, Ian Jackson, Wei Liu, Andrew Cooper
When using a PVH mode xenstore-stubdom the frame number of the console
should be a PFN instead of a MFN.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
V2:
- rename variable (Andrew Cooper)
---
tools/helpers/init-xenstore-domain.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/tools/helpers/init-xenstore-domain.c b/tools/helpers/init-xenstore-domain.c
index b205a79ee6..9457d0251b 100644
--- a/tools/helpers/init-xenstore-domain.c
+++ b/tools/helpers/init-xenstore-domain.c
@@ -30,7 +30,7 @@ static char *param;
static char *name = "Xenstore";
static int memory;
static int maxmem;
-static xen_pfn_t console_mfn;
+static xen_pfn_t console_gfn;
static xc_evtchn_port_or_error_t console_evtchn;
static struct option options[] = {
@@ -283,7 +283,9 @@ static int build(xc_interface *xch)
}
rv = 0;
- console_mfn = xc_dom_p2m(dom, dom->console_pfn);
+ console_gfn = (dom->container_type == XC_DOM_PV_CONTAINER)
+ ? xc_dom_p2m(dom, dom->console_pfn)
+ : dom->console_pfn;
err:
if ( dom )
@@ -528,7 +530,7 @@ int main(int argc, char** argv)
do_xs_write_dir_node(xsh, fe_path, "tty", "");
snprintf(buf, 16, "%d", console_evtchn);
do_xs_write_dir_node(xsh, fe_path, "port", buf);
- snprintf(buf, 16, "%ld", console_mfn);
+ snprintf(buf, 16, "%ld", console_gfn);
do_xs_write_dir_node(xsh, fe_path, "ring-ref", buf);
xs_close(xsh);
--
2.26.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v3 2/2] tools: set event channel HVM parameters in libxenguest
2021-12-08 8:47 [PATCH v3 0/2] tools/helpers: PVH xenstore-stubdom console fixes Juergen Gross
2021-12-08 8:47 ` [PATCH v3 1/2] tools/helpers: fix PVH xenstore-stubdom console parameters Juergen Gross
@ 2021-12-08 8:47 ` Juergen Gross
2021-12-08 13:43 ` Andrew Cooper
1 sibling, 1 reply; 9+ messages in thread
From: Juergen Gross @ 2021-12-08 8:47 UTC (permalink / raw)
To: xen-devel
Cc: Juergen Gross, Ian Jackson, Wei Liu, Anthony PERARD, Andrew Cooper
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 <andrew.cooper3@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
---
V3:
- replacement for former patch 2 (Andrew Cooper)
---
tools/libs/guest/xg_dom_x86.c | 6 ++++++
tools/libs/light/libxl_dom.c | 15 ++++++---------
2 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/tools/libs/guest/xg_dom_x86.c b/tools/libs/guest/xg_dom_x86.c
index b6e75afba2..9328fbf804 100644
--- a/tools/libs/guest/xg_dom_x86.c
+++ b/tools/libs/guest/xg_dom_x86.c
@@ -1866,6 +1866,12 @@ static int bootlate_hvm(struct xc_dom_image *dom)
munmap(hvm_info_page, PAGE_SIZE);
}
+ if ( xc_hvm_param_set(xch, domid, HVM_PARAM_CONSOLE_EVTCHN,
+ dom->console_evtchn) ||
+ xc_hvm_param_set(xch, domid, HVM_PARAM_STORE_EVTCHN,
+ dom->xenstore_evtchn) )
+ return -1;
+
return 0;
}
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);
- xc_hvm_param_set(handle, domid, HVM_PARAM_STORE_EVTCHN, store_evtchn);
- xc_hvm_param_set(handle, domid, HVM_PARAM_CONSOLE_EVTCHN, console_evtchn);
*store_mfn = str_mfn;
*console_mfn = cons_mfn;
@@ -1123,7 +1120,9 @@ int libxl__build_hvm(libxl__gc *gc, uint32_t domid,
dom->vga_hole_size = device_model ? LIBXL_VGA_HOLE_SIZE : 0;
dom->device_model = device_model;
dom->max_vcpus = info->max_vcpus;
+ dom->console_evtchn = state->console_port;
dom->console_domid = state->console_domid;
+ dom->xenstore_evtchn = state->store_port;
dom->xenstore_domid = state->store_domid;
rc = libxl__domain_device_construct_rdm(gc, d_config,
@@ -1169,10 +1168,8 @@ int libxl__build_hvm(libxl__gc *gc, uint32_t domid,
if (rc != 0)
goto out;
- rc = hvm_build_set_params(ctx->xch, domid, info, state->store_port,
- &state->store_mfn, state->console_port,
- &state->console_mfn, state->store_domid,
- state->console_domid);
+ rc = hvm_build_set_params(ctx->xch, domid, info, &state->store_mfn,
+ &state->console_mfn);
if (rc != 0) {
LOG(ERROR, "hvm build set params failed");
goto out;
--
2.26.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v3 2/2] tools: set event channel HVM parameters in libxenguest
2021-12-08 8:47 ` [PATCH v3 2/2] tools: set event channel HVM parameters in libxenguest Juergen Gross
@ 2021-12-08 13:43 ` Andrew Cooper
2021-12-08 14:22 ` Juergen Gross
0 siblings, 1 reply; 9+ messages in thread
From: Andrew Cooper @ 2021-12-08 13:43 UTC (permalink / raw)
To: Juergen Gross, xen-devel
Cc: Ian Jackson, Wei Liu, Anthony PERARD, Andrew Cooper
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 <andrew.cooper3@citrix.com>
> Signed-off-by: Juergen Gross <jgross@suse.com>
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);
With a suitable tweak to the commit message (probably just deleting the
final clause), Reivewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
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.
~Andrew
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 2/2] tools: set event channel HVM parameters in libxenguest
2021-12-08 13:43 ` Andrew Cooper
@ 2021-12-08 14:22 ` Juergen Gross
2021-12-08 15:54 ` Andrew Cooper
0 siblings, 1 reply; 9+ messages in thread
From: Juergen Gross @ 2021-12-08 14:22 UTC (permalink / raw)
To: Andrew Cooper, xen-devel
Cc: Ian Jackson, Wei Liu, Anthony PERARD, Andrew Cooper
[-- Attachment #1.1.1: Type: text/plain, Size: 3044 bytes --]
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 <andrew.cooper3@citrix.com>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>
> 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 <andrew.cooper3@citrix.com>
>
> 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
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3135 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 2/2] tools: set event channel HVM parameters in libxenguest
2021-12-08 14:22 ` Juergen Gross
@ 2021-12-08 15:54 ` Andrew Cooper
2021-12-08 15:57 ` Juergen Gross
0 siblings, 1 reply; 9+ messages in thread
From: Andrew Cooper @ 2021-12-08 15:54 UTC (permalink / raw)
To: Juergen Gross, xen-devel
Cc: Ian Jackson, Wei Liu, Anthony PERARD, Andrew Cooper
On 08/12/2021 14:22, Juergen Gross wrote:
> 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 <andrew.cooper3@citrix.com>
>>> Signed-off-by: Juergen Gross <jgross@suse.com>
>>
>> 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.
Yeah, but "allocated in the library call we're making" is not the same
as "caller has to allocate and pass details in".
I would not class the frames as "pre-allocated" in this context.
"allocated" sure, so perhaps "just like it is done for PV guests, and
the ring pages that libxenguest allocates" ?
>
>>
>> With a suitable tweak to the commit message (probably just deleting the
>> final clause), Reivewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>
>> 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.
Absolutely. This wasn't a request to merge more changes into this patch.
~Andrew
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 2/2] tools: set event channel HVM parameters in libxenguest
2021-12-08 15:54 ` Andrew Cooper
@ 2021-12-08 15:57 ` Juergen Gross
2021-12-08 16:02 ` Andrew Cooper
0 siblings, 1 reply; 9+ messages in thread
From: Juergen Gross @ 2021-12-08 15:57 UTC (permalink / raw)
To: Andrew Cooper, xen-devel
Cc: Ian Jackson, Wei Liu, Anthony PERARD, Andrew Cooper
[-- Attachment #1.1.1: Type: text/plain, Size: 1587 bytes --]
On 08.12.21 16:54, Andrew Cooper wrote:
> On 08/12/2021 14:22, Juergen Gross wrote:
>> 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 <andrew.cooper3@citrix.com>
>>>> Signed-off-by: Juergen Gross <jgross@suse.com>
>>>
>>> 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.
>
> Yeah, but "allocated in the library call we're making" is not the same
> as "caller has to allocate and pass details in".
>
> I would not class the frames as "pre-allocated" in this context.
> "allocated" sure, so perhaps "just like it is done for PV guests, and
> the ring pages that libxenguest allocates" ?
Fine with me.
Should I send another round, or can this be changed when committing?
Juergen
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3135 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 2/2] tools: set event channel HVM parameters in libxenguest
2021-12-08 15:57 ` Juergen Gross
@ 2021-12-08 16:02 ` Andrew Cooper
2021-12-08 16:34 ` Anthony PERARD
0 siblings, 1 reply; 9+ messages in thread
From: Andrew Cooper @ 2021-12-08 16:02 UTC (permalink / raw)
To: Juergen Gross, xen-devel
Cc: Ian Jackson, Wei Liu, Anthony PERARD, Andrew Cooper
On 08/12/2021 15:57, Juergen Gross wrote:
> On 08.12.21 16:54, Andrew Cooper wrote:
>> On 08/12/2021 14:22, Juergen Gross wrote:
>>> 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 <andrew.cooper3@citrix.com>
>>>>> Signed-off-by: Juergen Gross <jgross@suse.com>
>>>>
>>>> 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.
>>
>> Yeah, but "allocated in the library call we're making" is not the same
>> as "caller has to allocate and pass details in".
>>
>> I would not class the frames as "pre-allocated" in this context.
>> "allocated" sure, so perhaps "just like it is done for PV guests, and
>> the ring pages that libxenguest allocates" ?
>
> Fine with me.
>
> Should I send another round, or can this be changed when committing?
Fixed on commit. No need to resend just for this.
Question is whether Anthony has any view, or whether my R-by is good enough?
~Andrew
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v3 2/2] tools: set event channel HVM parameters in libxenguest
2021-12-08 16:02 ` Andrew Cooper
@ 2021-12-08 16:34 ` Anthony PERARD
0 siblings, 0 replies; 9+ messages in thread
From: Anthony PERARD @ 2021-12-08 16:34 UTC (permalink / raw)
To: Andrew Cooper
Cc: Juergen Gross, xen-devel, Ian Jackson, Wei Liu, Andrew Cooper
On Wed, Dec 08, 2021 at 04:02:17PM +0000, Andrew Cooper wrote:
> On 08/12/2021 15:57, Juergen Gross wrote:
> > On 08.12.21 16:54, Andrew Cooper wrote:
> >> On 08/12/2021 14:22, Juergen Gross wrote:
> >>> 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 <andrew.cooper3@citrix.com>
> >>>>> Signed-off-by: Juergen Gross <jgross@suse.com>
> >>>>
> >>>> 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.
> >>
> >> Yeah, but "allocated in the library call we're making" is not the same
> >> as "caller has to allocate and pass details in".
> >>
> >> I would not class the frames as "pre-allocated" in this context.
> >> "allocated" sure, so perhaps "just like it is done for PV guests, and
> >> the ring pages that libxenguest allocates" ?
> >
> > Fine with me.
> >
> > Should I send another round, or can this be changed when committing?
>
> Fixed on commit. No need to resend just for this.
>
> Question is whether Anthony has any view, or whether my R-by is good enough?
Patch looks good, so:
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
Thanks,
--
Anthony PERARD
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-12-08 16:35 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-08 8:47 [PATCH v3 0/2] tools/helpers: PVH xenstore-stubdom console fixes Juergen Gross
2021-12-08 8:47 ` [PATCH v3 1/2] tools/helpers: fix PVH xenstore-stubdom console parameters Juergen Gross
2021-12-08 8:47 ` [PATCH v3 2/2] tools: set event channel HVM parameters in libxenguest Juergen Gross
2021-12-08 13:43 ` Andrew Cooper
2021-12-08 14:22 ` Juergen Gross
2021-12-08 15:54 ` Andrew Cooper
2021-12-08 15:57 ` Juergen Gross
2021-12-08 16:02 ` Andrew Cooper
2021-12-08 16:34 ` Anthony PERARD
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.