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

[Andrew Cooper <amc96@xxxxxxxx>]

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@xxxxxxxxxx>
>>>> ---
>>>> CC: Wei Liu <wl@xxxxxxx>
>>>> CC: Anthony PERARD <anthony.perard@xxxxxxxxxx>
>>>> CC: Juergen Gross <jgross@xxxxxxxx>
>>>> ---
>>>>    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