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

[Connor Davis <connojdavis@gmail.com>]

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@gmail.com>
>> ---
>>   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