Re: [PATCH v2 4/5] xen: Add files needed for minimal riscv build

[Connor Davis <connojdavis@xxxxxxxxx>]

On 5/14/21 3:46 AM, Julien Grall wrote:
> Hi Connor,
>
> On 14/05/2021 05:17, Connor Davis wrote:
> > Add the minimum code required to get xen to build with
> > XEN_TARGET_ARCH=riscv64. It is minimal in the sense that every file and
> > function added is required for a successful build, given the .config
> > generated from riscv64_defconfig. The function implementations are just
> > stubs; actual implmentations will need to be added later.
> Thank you for the contribution. This is quite a large patch to review. 
> Could you consider to split in smaller one (I think Stefano suggested 
> one per header file or group of headers)? This would help to review 
> and find whether some bits can be moved in common.
Ok yes I will work on that.

> I would be happy to help moving some of the pieces.
Great!
> > Signed-off-by: Connor Davis <connojdavis@xxxxxxxxx>
> > ---
> >   config/riscv64.mk                        |   7 +
> >   xen/Makefile                             |   8 +-
> >   xen/arch/riscv/Kconfig                   |  54 ++++
> >   xen/arch/riscv/Kconfig.debug             |   0
> >   xen/arch/riscv/Makefile                  |  57 ++++
> >   xen/arch/riscv/README.source             |  19 ++
> >   xen/arch/riscv/Rules.mk                  |  13 +
> >   xen/arch/riscv/arch.mk                   |   7 +
> >   xen/arch/riscv/configs/riscv64_defconfig |  12 +
> >   xen/arch/riscv/delay.c                   |  16 +
> >   xen/arch/riscv/domain.c                  | 144 +++++++++
> >   xen/arch/riscv/domctl.c                  |  36 +++
> >   xen/arch/riscv/guestcopy.c               |  57 ++++
> >   xen/arch/riscv/head.S                    |   6 +
> >   xen/arch/riscv/irq.c                     |  78 +++++
> >   xen/arch/riscv/lib/Makefile              |   1 +
> >   xen/arch/riscv/lib/find_next_bit.c       | 284 +++++++++++++++++
> I quickly skimmed through the code and I think some of the file can be 
> made common such as this one.
Yep there is quite a bit of overlap from ARM
> [...]
>
> > diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
> > index 1708c36964..fd0b75677c 100644
> > --- a/xen/include/xen/domain.h
> > +++ b/xen/include/xen/domain.h
> > @@ -60,6 +60,7 @@ void arch_vcpu_destroy(struct vcpu *v);
> >   int map_vcpu_info(struct vcpu *v, unsigned long gfn, unsigned offset);
> >   void unmap_vcpu_info(struct vcpu *v);
> >   +struct xen_domctl_createdomain;
> This is needed because?
The build was failing without it. With the one commit-per-file approach 
we can probably avoid this.

Thanks,

Connor