Re: [PATCH V5 00/22] IOREQ feature (+ virtio-mmio) on Arm

[Oleksandr <olekstysh@xxxxxxxxx>]

On 26.01.21 02:14, Oleksandr wrote:

Hello, all

> On 26.01.21 01:20, Julien Grall wrote:
>
> Hi Julien, Stefano
>
> > On Mon, 25 Jan 2021 at 20:56, Stefano Stabellini 
> > <sstabellini@xxxxxxxxxx> wrote:
> > > Julien,
> > Hi,
> >
> > > This seems to be an arm randconfig failure:
> > >
> > > https://gitlab.com/xen-project/patchew/xen/-/pipelines/246632953
> > > https://gitlab.com/xen-project/patchew/xen/-/jobs/985455044
> > Thanks! The error is:
> >
> > #'target_mem_ref' not supported by expression#'memory.c: In function
> > 'do_memory_op':
> > memory.c:1210:18: error:  may be used uninitialized in this function
> > [-Werror=maybe-uninitialized]
> >   1210 |             rc = set_foreign_p2m_entry(currd, d, gfn_list[i],
> >        | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >   1211 | _mfn(mfn_list[i]));
> >        | ~~~~~~~~~~~~~~~~~~
> >
> > I found a few references online of the error message, but it is not
> > clear what it means. From a quick look at Oleksandr's branch, I also
> > can't spot anything unitialized. Any ideas?
> It seems that error happens if *both* CONFIG_GRANT_TABLE and 
> CONFIG_IOREQ_SERVER are disabled. Looks like that mfn_list is 
> initialized either in acquire_grant_table() or in acquire_ioreq_server().
> If these options disabled then corresponding helpers are just stubs, 
> so indeed that mfn_list gets uninitialized. But, I am not sure why gcc 
> complains about it as set_foreign_p2m_entry() is *not* going to be 
> called in that case???
This weird build error goes away if I simply add:

diff --git a/xen/common/memory.c b/xen/common/memory.c
index 33296e6..d1bd57b 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -1136,7 +1136,7 @@ static int acquire_resource(
      * moment since they are small, but if they need to grow in future
      * use-cases then per-CPU arrays or heap allocations may be required.
      */
-    xen_pfn_t mfn_list[32];
+    xen_pfn_t mfn_list[32] = {0};
     int rc;

     if ( !arch_acquire_resource_check(currd) )


Shall I make the corresponding patch?
But it is still unclear to me why the compiler doesn't recognize that 
*non-yet-uninitialized* mfn_list[] won't be used if both 
CONFIG_GRANT_TABLE and CONFIG_IOREQ_SERVER are not set...

--
Regards,

Oleksandr Tyshchenko