[PATCH] tools/libxl: Don't read STORE/CONSOLE_PFN from Xen

[PATCH] tools/libxl: Don't read STORE/CONSOLE_PFN from Xen

[Andrew Cooper]

The values are already available in dom->{console,xenstore}_pfn, just like on
the PV side of things.  No need to ask Xen.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Wei Liu <wl@xen.org>
CC: Anthony PERARD <anthony.perard@citrix.com>
CC: Juergen Gross <jgross@suse.com>
---
 tools/libs/light/libxl_dom.c | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/tools/libs/light/libxl_dom.c b/tools/libs/light/libxl_dom.c
index c9c24666cd04..03841243ab47 100644
--- a/tools/libs/light/libxl_dom.c
+++ b/tools/libs/light/libxl_dom.c
@@ -722,13 +722,10 @@ int libxl__build_pv(libxl__gc *gc, uint32_t domid,
 }
 
 static int hvm_build_set_params(xc_interface *handle, uint32_t domid,
-                                libxl_domain_build_info *info,
-                                unsigned long *store_mfn,
-                                unsigned long *console_mfn)
+                                libxl_domain_build_info *info)
 {
     struct hvm_info_table *va_hvm;
     uint8_t *va_map, sum;
-    uint64_t str_mfn, cons_mfn;
     int i;
 
     if (info->type == LIBXL_DOMAIN_TYPE_HVM) {
@@ -749,12 +746,6 @@ static int hvm_build_set_params(xc_interface *handle, uint32_t domid,
         munmap(va_map, XC_PAGE_SIZE);
     }
 
-    xc_hvm_param_get(handle, domid, HVM_PARAM_STORE_PFN, &str_mfn);
-    xc_hvm_param_get(handle, domid, HVM_PARAM_CONSOLE_PFN, &cons_mfn);
-
-    *store_mfn = str_mfn;
-    *console_mfn = cons_mfn;
-
     return 0;
 }
 
@@ -1168,12 +1159,14 @@ 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_mfn,
-                              &state->console_mfn);
+    rc = hvm_build_set_params(ctx->xch, domid, info);
     if (rc != 0) {
         LOG(ERROR, "hvm build set params failed");
         goto out;
     }
+
+    state->console_mfn = dom->console_pfn;
+    state->store_mfn = dom->xenstore_pfn;
     state->vuart_gfn = dom->vuart_gfn;
 
     rc = hvm_build_set_xs_values(gc, domid, dom, info);
-- 
2.11.0

Re: [PATCH] tools/libxl: Don't read STORE/CONSOLE_PFN from Xen

[Juergen Gross]

[-- Attachment #1.1.1: Type: text/plain, Size: 1430 bytes --]

On 09.12.21 18:07, Andrew Cooper wrote:
> The values are already available in dom->{console,xenstore}_pfn, just like on
> the PV side of things.  No need to ask Xen.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Wei Liu <wl@xen.org>
> CC: Anthony PERARD <anthony.perard@citrix.com>
> CC: Juergen Gross <jgross@suse.com>
> ---
>   tools/libs/light/libxl_dom.c | 17 +++++------------
>   1 file changed, 5 insertions(+), 12 deletions(-)
> 
> diff --git a/tools/libs/light/libxl_dom.c b/tools/libs/light/libxl_dom.c
> index c9c24666cd04..03841243ab47 100644
> --- a/tools/libs/light/libxl_dom.c
> +++ b/tools/libs/light/libxl_dom.c
> @@ -722,13 +722,10 @@ int libxl__build_pv(libxl__gc *gc, uint32_t domid,
>   }
>   
>   static int hvm_build_set_params(xc_interface *handle, uint32_t domid,
> -                                libxl_domain_build_info *info,
> -                                unsigned long *store_mfn,
> -                                unsigned long *console_mfn)
> +                                libxl_domain_build_info *info)
>   {
>       struct hvm_info_table *va_hvm;
>       uint8_t *va_map, sum;
> -    uint64_t str_mfn, cons_mfn;
>       int i;
>   
>       if (info->type == LIBXL_DOMAIN_TYPE_HVM) {

What about moving this if () to the only caller and renaming the
function from hvm_build_set_params() to hvm_set_info_table()?


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 --]

Re: [PATCH] tools/libxl: Don't read STORE/CONSOLE_PFN from Xen

[Andrew Cooper]

On 10/12/2021 11:16, Juergen Gross wrote:
> On 09.12.21 18:07, Andrew Cooper wrote:
>> The values are already available in dom->{console,xenstore}_pfn, just
>> like on
>> the PV side of things.  No need to ask Xen.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> ---
>> CC: Wei Liu <wl@xen.org>
>> CC: Anthony PERARD <anthony.perard@citrix.com>
>> CC: Juergen Gross <jgross@suse.com>
>> ---
>>   tools/libs/light/libxl_dom.c | 17 +++++------------
>>   1 file changed, 5 insertions(+), 12 deletions(-)
>>
>> diff --git a/tools/libs/light/libxl_dom.c b/tools/libs/light/libxl_dom.c
>> index c9c24666cd04..03841243ab47 100644
>> --- a/tools/libs/light/libxl_dom.c
>> +++ b/tools/libs/light/libxl_dom.c
>> @@ -722,13 +722,10 @@ int libxl__build_pv(libxl__gc *gc, uint32_t domid,
>>   }
>>     static int hvm_build_set_params(xc_interface *handle, uint32_t
>> domid,
>> -                                libxl_domain_build_info *info,
>> -                                unsigned long *store_mfn,
>> -                                unsigned long *console_mfn)
>> +                                libxl_domain_build_info *info)
>>   {
>>       struct hvm_info_table *va_hvm;
>>       uint8_t *va_map, sum;
>> -    uint64_t str_mfn, cons_mfn;
>>       int i;
>>         if (info->type == LIBXL_DOMAIN_TYPE_HVM) {
>
> What about moving this if () to the only caller and renaming the
> function from hvm_build_set_params() to hvm_set_info_table()?

Because I was hoping to delete it outright in a subsequent patch.

~Andrew

Re: [PATCH] tools/libxl: Don't read STORE/CONSOLE_PFN from Xen

[Juergen Gross]

[-- Attachment #1.1.1: Type: text/plain, Size: 1924 bytes --]

On 10.12.21 14:49, Andrew Cooper wrote:
> On 10/12/2021 11:16, Juergen Gross wrote:
>> On 09.12.21 18:07, Andrew Cooper wrote:
>>> The values are already available in dom->{console,xenstore}_pfn, just
>>> like on
>>> the PV side of things.  No need to ask Xen.
>>>
>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>> ---
>>> CC: Wei Liu <wl@xen.org>
>>> CC: Anthony PERARD <anthony.perard@citrix.com>
>>> CC: Juergen Gross <jgross@suse.com>
>>> ---
>>>    tools/libs/light/libxl_dom.c | 17 +++++------------
>>>    1 file changed, 5 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/tools/libs/light/libxl_dom.c b/tools/libs/light/libxl_dom.c
>>> index c9c24666cd04..03841243ab47 100644
>>> --- a/tools/libs/light/libxl_dom.c
>>> +++ b/tools/libs/light/libxl_dom.c
>>> @@ -722,13 +722,10 @@ int libxl__build_pv(libxl__gc *gc, uint32_t domid,
>>>    }
>>>      static int hvm_build_set_params(xc_interface *handle, uint32_t
>>> domid,
>>> -                                libxl_domain_build_info *info,
>>> -                                unsigned long *store_mfn,
>>> -                                unsigned long *console_mfn)
>>> +                                libxl_domain_build_info *info)
>>>    {
>>>        struct hvm_info_table *va_hvm;
>>>        uint8_t *va_map, sum;
>>> -    uint64_t str_mfn, cons_mfn;
>>>        int i;
>>>          if (info->type == LIBXL_DOMAIN_TYPE_HVM) {
>>
>> What about moving this if () to the only caller and renaming the
>> function from hvm_build_set_params() to hvm_set_info_table()?
> 
> Because I was hoping to delete it outright in a subsequent patch.

I'd suggest to either do the renaming or to add that subsequent patch
making this a small series.


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 --]

Re: [PATCH] tools/libxl: Don't read STORE/CONSOLE_PFN from Xen

[Andrew Cooper]

On 10/12/2021 14:08, Juergen Gross wrote:
> On 10.12.21 14:49, Andrew Cooper wrote:
>> On 10/12/2021 11:16, Juergen Gross wrote:
>>> On 09.12.21 18:07, Andrew Cooper wrote:
>>>> The values are already available in dom->{console,xenstore}_pfn, just
>>>> like on
>>>> the PV side of things.  No need to ask Xen.
>>>>
>>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>>> ---
>>>> CC: Wei Liu <wl@xen.org>
>>>> CC: Anthony PERARD <anthony.perard@citrix.com>
>>>> CC: Juergen Gross <jgross@suse.com>
>>>> ---
>>>>    tools/libs/light/libxl_dom.c | 17 +++++------------
>>>>    1 file changed, 5 insertions(+), 12 deletions(-)
>>>>
>>>> diff --git a/tools/libs/light/libxl_dom.c
>>>> b/tools/libs/light/libxl_dom.c
>>>> index c9c24666cd04..03841243ab47 100644
>>>> --- a/tools/libs/light/libxl_dom.c
>>>> +++ b/tools/libs/light/libxl_dom.c
>>>> @@ -722,13 +722,10 @@ int libxl__build_pv(libxl__gc *gc, uint32_t
>>>> domid,
>>>>    }
>>>>      static int hvm_build_set_params(xc_interface *handle, uint32_t
>>>> domid,
>>>> -                                libxl_domain_build_info *info,
>>>> -                                unsigned long *store_mfn,
>>>> -                                unsigned long *console_mfn)
>>>> +                                libxl_domain_build_info *info)
>>>>    {
>>>>        struct hvm_info_table *va_hvm;
>>>>        uint8_t *va_map, sum;
>>>> -    uint64_t str_mfn, cons_mfn;
>>>>        int i;
>>>>          if (info->type == LIBXL_DOMAIN_TYPE_HVM) {
>>>
>>> What about moving this if () to the only caller and renaming the
>>> function from hvm_build_set_params() to hvm_set_info_table()?
>>
>> Because I was hoping to delete it outright in a subsequent patch.
>
> I'd suggest to either do the renaming or to add that subsequent patch
> making this a small series.

That's a separate task, which I don't have time to untangle right now.

I don't think it is worth delaying this improvement for a future
only-tangentially-related change.

~Andrew

Re: [PATCH] tools/libxl: Don't read STORE/CONSOLE_PFN from Xen

[Anthony PERARD]

On Thu, Dec 09, 2021 at 05:07:52PM +0000, Andrew Cooper wrote:
> The values are already available in dom->{console,xenstore}_pfn, just like on
> the PV side of things.  No need to ask Xen.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>

Thanks,

-- 
Anthony PERARD