Re: [PATCH v1 1/5] xen/riscv: add stub for share_xen_page_with_guest()

[oleksii.kurochko@xxxxxxxxx]

On Thu, 2024-10-17 at 16:51 +0200, Jan Beulich wrote:
> On 16.10.2024 11:15, Oleksii Kurochko wrote:
> > To avoid the following linkage fail the stub for
> > share_xen_page_with_guest()
> > is introduced:
> 
> What do you intend to express with "is introduced"? Is there a
> problem now?
> Would there be a problem with subsequent changes? I'm entirely fine
> with
> adding that stub, but the description should make clear why /when
> exactly
> it's needed.
I mentioned that in the cover letter:
```
   Also, after patch 3 ("xen/riscv: introduce setup_mm()") of this
   patch series,
   the linkage error mentioned in patch 1 ("xen/riscv: add stub for
   share_xen_page_with_guest()") began to occur, so patch 1 addresses
   this issue.
```
I thought it would be the better then just mention in the commit
message that.

Will it be fine to mention instead:
```
   Introduction of setup memory management function will require
   share_xen_page_with_guest() defined, at least, as a stub otherwise
   the following linkage error will occur...
```

Perhaps it would be better just to merge these changes with patch 3 and
add an explanation in patch 3 commit message.

~ Oleksii
> >   riscv64-linux-gnu-ld: prelink.o: in function `tasklet_kill':
> >   /build/xen/common/tasklet.c:176: undefined reference to
> > `share_xen_page_with_guest'
> >   riscv64-linux-gnu-ld: ./.xen-syms.0: hidden symbol
> > `share_xen_page_with_guest' isn't defined
> >   riscv64-linux-gnu-ld: final link failed: bad value
> > 
> > $ find . -name \*.o | while read F; do nm $F | grep
> > share_xen_page_with_guest && echo $F; done
> >                  U share_xen_page_with_guest
> > ./xen/common/built_in.o
> >                  U share_xen_page_with_guest
> > ./xen/common/trace.o
> >                  U share_xen_page_with_guest
> > ./xen/prelink.o
> > 
> > Despite the linker fingering tasklet.c (very likely a toolchain
> > bug),
> > it's trace.o which has the undefined reference.
> > 
> > Looking at trace.i, there is call of share_xen_page_with_guest() in
> > share_xen_page_with_privileged_guests() from asm/mm.h.
> > 
> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> > ---
> >  xen/arch/riscv/stubs.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/xen/arch/riscv/stubs.c b/xen/arch/riscv/stubs.c
> > index 5951b0ce91..c9a590b225 100644
> > --- a/xen/arch/riscv/stubs.c
> > +++ b/xen/arch/riscv/stubs.c
> > @@ -2,7 +2,9 @@
> >  #include <xen/cpumask.h>
> >  #include <xen/domain.h>
> >  #include <xen/irq.h>
> > +#include <xen/mm.h>
> >  #include <xen/nodemask.h>
> > +#include <xen/sched.h>
> >  #include <xen/sections.h>
> >  #include <xen/time.h>
> >  #include <public/domctl.h>
> > @@ -409,3 +411,11 @@ unsigned long get_upper_mfn_bound(void)
> >  {
> >      BUG_ON("unimplemented");
> >  }
> > +
> > +/* mm.c */
> > +
> > +void share_xen_page_with_guest(struct page_info *page, struct
> > domain *d,
> > +                               enum XENSHARE_flags flags)
> > +{
> > +    BUG_ON("unimplemented");
> > +}
>