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

2021-12-14 Thread Anthony PERARD 
On Thu, Dec 09, 2021 at 05:07:52PM +, 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 

Reviewed-by: Anthony PERARD 

Thanks,

-- 
Anthony PERARD

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

2021-12-13 Thread 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 
 ---
 CC: Wei Liu 
 CC: Anthony PERARD 
 CC: Juergen Gross 
 ---
    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

2021-12-10 Thread Juergen Gross 

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 
---
CC: Wei Liu 
CC: Anthony PERARD 
CC: Juergen Gross 
---
   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


OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature

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

2021-12-10 Thread 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 
>> ---
>> CC: Wei Liu 
>> CC: Anthony PERARD 
>> CC: Juergen Gross 
>> ---
>>   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

2021-12-10 Thread Juergen Gross 

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 
---
CC: Wei Liu 
CC: Anthony PERARD 
CC: Juergen Gross 
---
  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


OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature

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

2021-12-09 Thread 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 
---
CC: Wei Liu 
CC: Anthony PERARD 
CC: Juergen Gross 
---
 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