[PATCH] libxc/arm: initialise p2m_size to make gcc happy

[PATCH] libxc/arm: initialise p2m_size to make gcc happy

[Wei Liu]

Gcc with -O3 failed to spot the loop to initialise p2m_size runs at
least once.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxc/xc_dom_arm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c
index 5b9eca6087..97d08f055a 100644
--- a/tools/libxc/xc_dom_arm.c
+++ b/tools/libxc/xc_dom_arm.c
@@ -395,7 +395,7 @@ static int meminit(struct xc_dom_image *dom)
     const uint64_t modsize = dtb_size + ramdisk_size;
     const uint64_t ram128mb = bankbase[0] + (128<<20);
 
-    xen_pfn_t p2m_size;
+    xen_pfn_t p2m_size = 0; /* make gcc -O3 happy */
     uint64_t bank0end;
 
     assert(dom->rambase_pfn << XC_PAGE_SHIFT == bankbase[0]);
@@ -438,6 +438,7 @@ static int meminit(struct xc_dom_image *dom)
 
     assert(dom->rambank_size[0] != 0);
     assert(ramsize == 0); /* Too much RAM is rejected above */
+    assert(p2m_size != 0);
 
     dom->p2m_size = p2m_size;
     dom->p2m_host = xc_dom_malloc(dom, sizeof(xen_pfn_t) * p2m_size);
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] libxc/arm: initialise p2m_size to make gcc happy

[Andrew Cooper]

On 14/03/18 12:32, Wei Liu wrote:
> Gcc with -O3 failed to spot the loop to initialise p2m_size runs at
> least once.

It is -Og in this case, rather than -O3.  My -O3 comment on the thread
was for the more generic cases.

~Andrew

>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
>  tools/libxc/xc_dom_arm.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c
> index 5b9eca6087..97d08f055a 100644
> --- a/tools/libxc/xc_dom_arm.c
> +++ b/tools/libxc/xc_dom_arm.c
> @@ -395,7 +395,7 @@ static int meminit(struct xc_dom_image *dom)
>      const uint64_t modsize = dtb_size + ramdisk_size;
>      const uint64_t ram128mb = bankbase[0] + (128<<20);
>  
> -    xen_pfn_t p2m_size;
> +    xen_pfn_t p2m_size = 0; /* make gcc -O3 happy */
>      uint64_t bank0end;
>  
>      assert(dom->rambase_pfn << XC_PAGE_SHIFT == bankbase[0]);
> @@ -438,6 +438,7 @@ static int meminit(struct xc_dom_image *dom)
>  
>      assert(dom->rambank_size[0] != 0);
>      assert(ramsize == 0); /* Too much RAM is rejected above */
> +    assert(p2m_size != 0);
>  
>      dom->p2m_size = p2m_size;
>      dom->p2m_host = xc_dom_malloc(dom, sizeof(xen_pfn_t) * p2m_size);


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] libxc/arm: initialise p2m_size to make gcc happy

[Wei Liu]

On Wed, Mar 14, 2018 at 12:35:48PM +0000, Andrew Cooper wrote:
> On 14/03/18 12:32, Wei Liu wrote:
> > Gcc with -O3 failed to spot the loop to initialise p2m_size runs at
> > least once.
> 
> It is -Og in this case, rather than -O3.  My -O3 comment on the thread
> was for the more generic cases.

OK. I will fix the commit message and code comment.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] libxc/arm: initialise p2m_size to make gcc happy

[Julien Grall]

Hi,

On 03/14/2018 12:32 PM, Wei Liu wrote:
> Gcc with -O3 failed to spot the loop to initialise p2m_size runs at
> least once.

Aside, Andrew's comment the patch looks okay. But I am wondering why we 
need to allocate p2m_host for Arm?

 From a quick look I have seen no real user except 
xc_dom_update_guest_p2m that can cope with p2m_host = NULL.

Cheers,

> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
>   tools/libxc/xc_dom_arm.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c
> index 5b9eca6087..97d08f055a 100644
> --- a/tools/libxc/xc_dom_arm.c
> +++ b/tools/libxc/xc_dom_arm.c
> @@ -395,7 +395,7 @@ static int meminit(struct xc_dom_image *dom)
>       const uint64_t modsize = dtb_size + ramdisk_size;
>       const uint64_t ram128mb = bankbase[0] + (128<<20);
>   
> -    xen_pfn_t p2m_size;
> +    xen_pfn_t p2m_size = 0; /* make gcc -O3 happy */
>       uint64_t bank0end;
>   
>       assert(dom->rambase_pfn << XC_PAGE_SHIFT == bankbase[0]);
> @@ -438,6 +438,7 @@ static int meminit(struct xc_dom_image *dom)
>   
>       assert(dom->rambank_size[0] != 0);
>       assert(ramsize == 0); /* Too much RAM is rejected above */
> +    assert(p2m_size != 0);
>   
>       dom->p2m_size = p2m_size;
>       dom->p2m_host = xc_dom_malloc(dom, sizeof(xen_pfn_t) * p2m_size);
> 

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] libxc/arm: initialise p2m_size to make gcc happy

[Jan Beulich]

>>> On 14.03.18 at 13:35, <andrew.cooper3@citrix.com> wrote:
> On 14/03/18 12:32, Wei Liu wrote:
>> Gcc with -O3 failed to spot the loop to initialise p2m_size runs at
>> least once.
> 
> It is -Og in this case, rather than -O3.  My -O3 comment on the thread
> was for the more generic cases.

Yeah, it is likely the fact that gcc does _less_ optimization with -Og
that makes it no longer spot that the variable can't be used
uninitialized.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] libxc/arm: initialise p2m_size to make gcc happy

[Wei Liu]

On Wed, Mar 14, 2018 at 01:27:37PM +0000, Julien Grall wrote:
> Hi,
> 
> On 03/14/2018 12:32 PM, Wei Liu wrote:
> > Gcc with -O3 failed to spot the loop to initialise p2m_size runs at
> > least once.
> 
> Aside, Andrew's comment the patch looks okay. But I am wondering why we need
> to allocate p2m_host for Arm?
> 

Not sure, really. :-)

> From a quick look I have seen no real user except xc_dom_update_guest_p2m
> that can cope with p2m_host = NULL.
> 

Maybe it is to work around this limitation in code.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] libxc/arm: initialise p2m_size to make gcc happy

[Julien Grall]

Hi,

On 03/21/2018 04:43 PM, Wei Liu wrote:
> On Wed, Mar 14, 2018 at 01:27:37PM +0000, Julien Grall wrote:
>> Hi,
>>
>> On 03/14/2018 12:32 PM, Wei Liu wrote:
>>> Gcc with -O3 failed to spot the loop to initialise p2m_size runs at
>>> least once.
>>
>> Aside, Andrew's comment the patch looks okay. But I am wondering why we need
>> to allocate p2m_host for Arm?
>>
> 
> Not sure, really. :-)
> 
>>  From a quick look I have seen no real user except xc_dom_update_guest_p2m
>> that can cope with p2m_host = NULL.
>>
> 
> Maybe it is to work around this limitation in code.

I will have a look at it. Meanwhile, I guess it is better to make GCC 
happy with that patch.

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel