diff mbox

[v3,01/17] libxl/arm: Factor out codes for generating DTB

Message ID 1467688367-17320-2-git-send-email-zhaoshenglong@huawei.com (mailing list archive)
State New, archived
Headers show

Commit Message

Shannon Zhao July 5, 2016, 3:12 a.m. UTC
From: Shannon Zhao <shannon.zhao@linaro.org>

Factor out codes for generating DTB to prepare for adding ACPI tables
generation codes.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl_arm.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

Comments

Wei Liu July 7, 2016, 3:41 p.m. UTC | #1
On Tue, Jul 05, 2016 at 11:12:31AM +0800, Shannon Zhao wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> Factor out codes for generating DTB to prepare for adding ACPI tables
> generation codes.
> 
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> Acked-by: Wei Liu <wei.liu2@citrix.com>
> ---
>  tools/libxl/libxl_arm.c | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
> index ddd80aa..4a57dd7 100644
> --- a/tools/libxl/libxl_arm.c
> +++ b/tools/libxl/libxl_arm.c
> @@ -747,10 +747,9 @@ static int copy_partial_fdt(libxl__gc *gc, void *fdt, void *pfdt)
>  
>  #define FDT_MAX_SIZE (1<<20)
>  
> -int libxl__arch_domain_init_hw_description(libxl__gc *gc,
> -                                           libxl_domain_build_info *info,
> -                                           libxl__domain_build_state *state,
> -                                           struct xc_dom_image *dom)
> +static int libxl__prepare_dtb(libxl__gc *gc, libxl_domain_build_info *info,
> +                       libxl__domain_build_state *state,
> +                       struct xc_dom_image *dom)

I've queued this up for committing and will fix the indentation as I go
along.
Boris Ostrovsky July 7, 2016, 3:48 p.m. UTC | #2
On 07/07/2016 11:41 AM, Wei Liu wrote:
> On Tue, Jul 05, 2016 at 11:12:31AM +0800, Shannon Zhao wrote:
>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>
>> Factor out codes for generating DTB to prepare for adding ACPI tables
>> generation codes.
>>
>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>> Acked-by: Wei Liu <wei.liu2@citrix.com>
>> ---
>>  tools/libxl/libxl_arm.c | 18 ++++++++++++------
>>  1 file changed, 12 insertions(+), 6 deletions(-)
>>
>> diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
>> index ddd80aa..4a57dd7 100644
>> --- a/tools/libxl/libxl_arm.c
>> +++ b/tools/libxl/libxl_arm.c
>> @@ -747,10 +747,9 @@ static int copy_partial_fdt(libxl__gc *gc, void *fdt, void *pfdt)
>>  
>>  #define FDT_MAX_SIZE (1<<20)
>>  
>> -int libxl__arch_domain_init_hw_description(libxl__gc *gc,
>> -                                           libxl_domain_build_info *info,
>> -                                           libxl__domain_build_state *state,
>> -                                           struct xc_dom_image *dom)
>> +static int libxl__prepare_dtb(libxl__gc *gc, libxl_domain_build_info *info,
>> +                       libxl__domain_build_state *state,
>> +                       struct xc_dom_image *dom)
> I've queued this up for committing and will fix the indentation as I go
> along.


I don't think this can be ready for committing since it sits on top of
my not-yet-reviewed series (which is pretty much guaranteed to require a
new spin).


-boris
Julien Grall July 7, 2016, 4:06 p.m. UTC | #3
On 07/07/16 16:48, Boris Ostrovsky wrote:
> On 07/07/2016 11:41 AM, Wei Liu wrote:
>> On Tue, Jul 05, 2016 at 11:12:31AM +0800, Shannon Zhao wrote:
>>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>>
>>> Factor out codes for generating DTB to prepare for adding ACPI tables
>>> generation codes.
>>>
>>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>>> Acked-by: Wei Liu <wei.liu2@citrix.com>
>>> ---
>>>   tools/libxl/libxl_arm.c | 18 ++++++++++++------
>>>   1 file changed, 12 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
>>> index ddd80aa..4a57dd7 100644
>>> --- a/tools/libxl/libxl_arm.c
>>> +++ b/tools/libxl/libxl_arm.c
>>> @@ -747,10 +747,9 @@ static int copy_partial_fdt(libxl__gc *gc, void *fdt, void *pfdt)
>>>
>>>   #define FDT_MAX_SIZE (1<<20)
>>>
>>> -int libxl__arch_domain_init_hw_description(libxl__gc *gc,
>>> -                                           libxl_domain_build_info *info,
>>> -                                           libxl__domain_build_state *state,
>>> -                                           struct xc_dom_image *dom)
>>> +static int libxl__prepare_dtb(libxl__gc *gc, libxl_domain_build_info *info,
>>> +                       libxl__domain_build_state *state,
>>> +                       struct xc_dom_image *dom)
>> I've queued this up for committing and will fix the indentation as I go
>> along.
>
>
> I don't think this can be ready for committing since it sits on top of
> my not-yet-reviewed series (which is pretty much guaranteed to require a
> new spin).

This is only used by the ARM code which you don't modify. So I don't see 
any issue to commit this patch.

Regards,
Wei Liu July 7, 2016, 4:08 p.m. UTC | #4
On Thu, Jul 07, 2016 at 11:48:49AM -0400, Boris Ostrovsky wrote:
> On 07/07/2016 11:41 AM, Wei Liu wrote:
> > On Tue, Jul 05, 2016 at 11:12:31AM +0800, Shannon Zhao wrote:
> >> From: Shannon Zhao <shannon.zhao@linaro.org>
> >>
> >> Factor out codes for generating DTB to prepare for adding ACPI tables
> >> generation codes.
> >>
> >> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> >> Acked-by: Wei Liu <wei.liu2@citrix.com>
> >> ---
> >>  tools/libxl/libxl_arm.c | 18 ++++++++++++------
> >>  1 file changed, 12 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
> >> index ddd80aa..4a57dd7 100644
> >> --- a/tools/libxl/libxl_arm.c
> >> +++ b/tools/libxl/libxl_arm.c
> >> @@ -747,10 +747,9 @@ static int copy_partial_fdt(libxl__gc *gc, void *fdt, void *pfdt)
> >>  
> >>  #define FDT_MAX_SIZE (1<<20)
> >>  
> >> -int libxl__arch_domain_init_hw_description(libxl__gc *gc,
> >> -                                           libxl_domain_build_info *info,
> >> -                                           libxl__domain_build_state *state,
> >> -                                           struct xc_dom_image *dom)
> >> +static int libxl__prepare_dtb(libxl__gc *gc, libxl_domain_build_info *info,
> >> +                       libxl__domain_build_state *state,
> >> +                       struct xc_dom_image *dom)
> > I've queued this up for committing and will fix the indentation as I go
> > along.
> 
> 
> I don't think this can be ready for committing since it sits on top of
> my not-yet-reviewed series (which is pretty much guaranteed to require a
> new spin).
> 

I thought this patch posted long time ago. But things might have
changed. I will wait.

> 
> -boris
> 
> 
>
Wei Liu July 7, 2016, 4:09 p.m. UTC | #5
On Thu, Jul 07, 2016 at 05:06:11PM +0100, Julien Grall wrote:
> 
> 
> On 07/07/16 16:48, Boris Ostrovsky wrote:
> >On 07/07/2016 11:41 AM, Wei Liu wrote:
> >>On Tue, Jul 05, 2016 at 11:12:31AM +0800, Shannon Zhao wrote:
> >>>From: Shannon Zhao <shannon.zhao@linaro.org>
> >>>
> >>>Factor out codes for generating DTB to prepare for adding ACPI tables
> >>>generation codes.
> >>>
> >>>Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> >>>Acked-by: Wei Liu <wei.liu2@citrix.com>
> >>>---
> >>>  tools/libxl/libxl_arm.c | 18 ++++++++++++------
> >>>  1 file changed, 12 insertions(+), 6 deletions(-)
> >>>
> >>>diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
> >>>index ddd80aa..4a57dd7 100644
> >>>--- a/tools/libxl/libxl_arm.c
> >>>+++ b/tools/libxl/libxl_arm.c
> >>>@@ -747,10 +747,9 @@ static int copy_partial_fdt(libxl__gc *gc, void *fdt, void *pfdt)
> >>>
> >>>  #define FDT_MAX_SIZE (1<<20)
> >>>
> >>>-int libxl__arch_domain_init_hw_description(libxl__gc *gc,
> >>>-                                           libxl_domain_build_info *info,
> >>>-                                           libxl__domain_build_state *state,
> >>>-                                           struct xc_dom_image *dom)
> >>>+static int libxl__prepare_dtb(libxl__gc *gc, libxl_domain_build_info *info,
> >>>+                       libxl__domain_build_state *state,
> >>>+                       struct xc_dom_image *dom)
> >>I've queued this up for committing and will fix the indentation as I go
> >>along.
> >
> >
> >I don't think this can be ready for committing since it sits on top of
> >my not-yet-reviewed series (which is pretty much guaranteed to require a
> >new spin).
> 
> This is only used by the ARM code which you don't modify. So I don't see any
> issue to commit this patch.

Ah, so I can go ahead after all.

> 
> Regards,
> 
> 
> -- 
> Julien Grall
Boris Ostrovsky July 7, 2016, 4:12 p.m. UTC | #6
On 07/07/2016 12:06 PM, Julien Grall wrote:
>
>
> On 07/07/16 16:48, Boris Ostrovsky wrote:
>> On 07/07/2016 11:41 AM, Wei Liu wrote:
>>> On Tue, Jul 05, 2016 at 11:12:31AM +0800, Shannon Zhao wrote:
>>>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>>>
>>>> Factor out codes for generating DTB to prepare for adding ACPI tables
>>>> generation codes.
>>>>
>>>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>>>> Acked-by: Wei Liu <wei.liu2@citrix.com>
>>>> ---
>>>>   tools/libxl/libxl_arm.c | 18 ++++++++++++------
>>>>   1 file changed, 12 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
>>>> index ddd80aa..4a57dd7 100644
>>>> --- a/tools/libxl/libxl_arm.c
>>>> +++ b/tools/libxl/libxl_arm.c
>>>> @@ -747,10 +747,9 @@ static int copy_partial_fdt(libxl__gc *gc,
>>>> void *fdt, void *pfdt)
>>>>
>>>>   #define FDT_MAX_SIZE (1<<20)
>>>>
>>>> -int libxl__arch_domain_init_hw_description(libxl__gc *gc,
>>>> -                                           libxl_domain_build_info
>>>> *info,
>>>> -                                          
>>>> libxl__domain_build_state *state,
>>>> -                                           struct xc_dom_image *dom)
>>>> +static int libxl__prepare_dtb(libxl__gc *gc,
>>>> libxl_domain_build_info *info,
>>>> +                       libxl__domain_build_state *state,
>>>> +                       struct xc_dom_image *dom)
>>> I've queued this up for committing and will fix the indentation as I go
>>> along.
>>
>>
>> I don't think this can be ready for committing since it sits on top of
>> my not-yet-reviewed series (which is pretty much guaranteed to require a
>> new spin).
>
> This is only used by the ARM code which you don't modify. So I don't
> see any issue to commit this patch.

I thought Wei was referring to the whole series being prepared for
committing. If he was talking only about this patch (and possibly
selected other patches) then yes, there is no dependency here. I know
that further patches in this series want to see tools/libacpi, for
example, and that is not ready.

-boris
Julien Grall July 7, 2016, 4:15 p.m. UTC | #7
On 07/07/16 17:12, Boris Ostrovsky wrote:
> On 07/07/2016 12:06 PM, Julien Grall wrote:
>>
>>
>> On 07/07/16 16:48, Boris Ostrovsky wrote:
>>> On 07/07/2016 11:41 AM, Wei Liu wrote:
>>>> On Tue, Jul 05, 2016 at 11:12:31AM +0800, Shannon Zhao wrote:
>>>>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>>>>
>>>>> Factor out codes for generating DTB to prepare for adding ACPI tables
>>>>> generation codes.
>>>>>
>>>>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>>>>> Acked-by: Wei Liu <wei.liu2@citrix.com>
>>>>> ---
>>>>>    tools/libxl/libxl_arm.c | 18 ++++++++++++------
>>>>>    1 file changed, 12 insertions(+), 6 deletions(-)
>>>>>
>>>>> diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
>>>>> index ddd80aa..4a57dd7 100644
>>>>> --- a/tools/libxl/libxl_arm.c
>>>>> +++ b/tools/libxl/libxl_arm.c
>>>>> @@ -747,10 +747,9 @@ static int copy_partial_fdt(libxl__gc *gc,
>>>>> void *fdt, void *pfdt)
>>>>>
>>>>>    #define FDT_MAX_SIZE (1<<20)
>>>>>
>>>>> -int libxl__arch_domain_init_hw_description(libxl__gc *gc,
>>>>> -                                           libxl_domain_build_info
>>>>> *info,
>>>>> -
>>>>> libxl__domain_build_state *state,
>>>>> -                                           struct xc_dom_image *dom)
>>>>> +static int libxl__prepare_dtb(libxl__gc *gc,
>>>>> libxl_domain_build_info *info,
>>>>> +                       libxl__domain_build_state *state,
>>>>> +                       struct xc_dom_image *dom)
>>>> I've queued this up for committing and will fix the indentation as I go
>>>> along.
>>>
>>>
>>> I don't think this can be ready for committing since it sits on top of
>>> my not-yet-reviewed series (which is pretty much guaranteed to require a
>>> new spin).
>>
>> This is only used by the ARM code which you don't modify. So I don't
>> see any issue to commit this patch.
>
> I thought Wei was referring to the whole series being prepared for
> committing. If he was talking only about this patch (and possibly
> selected other patches) then yes, there is no dependency here. I know
> that further patches in this series want to see tools/libacpi, for
> example, and that is not ready.

I know about this. I don't think this series is ready to be fully 
committed due to some the disagreement such as where to handle the ACPI 
blob.

However, it would be nice to get trivial patch to slim down the size of 
the series.

Cheers,
Wei Liu July 7, 2016, 4:16 p.m. UTC | #8
On Thu, Jul 07, 2016 at 12:12:48PM -0400, Boris Ostrovsky wrote:
> On 07/07/2016 12:06 PM, Julien Grall wrote:
> >
> >
> > On 07/07/16 16:48, Boris Ostrovsky wrote:
> >> On 07/07/2016 11:41 AM, Wei Liu wrote:
> >>> On Tue, Jul 05, 2016 at 11:12:31AM +0800, Shannon Zhao wrote:
> >>>> From: Shannon Zhao <shannon.zhao@linaro.org>
> >>>>
> >>>> Factor out codes for generating DTB to prepare for adding ACPI tables
> >>>> generation codes.
> >>>>
> >>>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> >>>> Acked-by: Wei Liu <wei.liu2@citrix.com>
> >>>> ---
> >>>>   tools/libxl/libxl_arm.c | 18 ++++++++++++------
> >>>>   1 file changed, 12 insertions(+), 6 deletions(-)
> >>>>
> >>>> diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
> >>>> index ddd80aa..4a57dd7 100644
> >>>> --- a/tools/libxl/libxl_arm.c
> >>>> +++ b/tools/libxl/libxl_arm.c
> >>>> @@ -747,10 +747,9 @@ static int copy_partial_fdt(libxl__gc *gc,
> >>>> void *fdt, void *pfdt)
> >>>>
> >>>>   #define FDT_MAX_SIZE (1<<20)
> >>>>
> >>>> -int libxl__arch_domain_init_hw_description(libxl__gc *gc,
> >>>> -                                           libxl_domain_build_info
> >>>> *info,
> >>>> -                                          
> >>>> libxl__domain_build_state *state,
> >>>> -                                           struct xc_dom_image *dom)
> >>>> +static int libxl__prepare_dtb(libxl__gc *gc,
> >>>> libxl_domain_build_info *info,
> >>>> +                       libxl__domain_build_state *state,
> >>>> +                       struct xc_dom_image *dom)
> >>> I've queued this up for committing and will fix the indentation as I go
> >>> along.
> >>
> >>
> >> I don't think this can be ready for committing since it sits on top of
> >> my not-yet-reviewed series (which is pretty much guaranteed to require a
> >> new spin).
> >
> > This is only used by the ARM code which you don't modify. So I don't
> > see any issue to commit this patch.
> 
> I thought Wei was referring to the whole series being prepared for
> committing. If he was talking only about this patch (and possibly
> selected other patches) then yes, there is no dependency here. I know
> that further patches in this series want to see tools/libacpi, for
> example, and that is not ready.
> 

Oh yes, sure. This patch series is not yet ready to go in. :-)

I was only about to commit this first patch to reduce Shannon's patch
queue length.

> -boris
> 
>
diff mbox

Patch

diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
index ddd80aa..4a57dd7 100644
--- a/tools/libxl/libxl_arm.c
+++ b/tools/libxl/libxl_arm.c
@@ -747,10 +747,9 @@  static int copy_partial_fdt(libxl__gc *gc, void *fdt, void *pfdt)
 
 #define FDT_MAX_SIZE (1<<20)
 
-int libxl__arch_domain_init_hw_description(libxl__gc *gc,
-                                           libxl_domain_build_info *info,
-                                           libxl__domain_build_state *state,
-                                           struct xc_dom_image *dom)
+static int libxl__prepare_dtb(libxl__gc *gc, libxl_domain_build_info *info,
+                       libxl__domain_build_state *state,
+                       struct xc_dom_image *dom)
 {
     void *fdt = NULL;
     void *pfdt = NULL;
@@ -764,8 +763,6 @@  int libxl__arch_domain_init_hw_description(libxl__gc *gc,
     /* convenience aliases */
     xc_domain_configuration_t *xc_config = &state->config;
 
-    assert(info->type == LIBXL_DOMAIN_TYPE_PV);
-
     vers = libxl_get_version_info(CTX);
     if (vers == NULL) return ERROR_FAIL;
 
@@ -883,6 +880,15 @@  out:
     return rc;
 }
 
+int libxl__arch_domain_init_hw_description(libxl__gc *gc,
+                                           libxl_domain_build_info *info,
+                                           libxl__domain_build_state *state,
+                                           struct xc_dom_image *dom)
+{
+    assert(info->type == LIBXL_DOMAIN_TYPE_PV);
+    return libxl__prepare_dtb(gc, info, state, dom);
+}
+
 static void finalise_one_memory_node(libxl__gc *gc, void *fdt,
                                      uint64_t base, uint64_t size)
 {