Re: [PATCH V4 23/24] libxl: Introduce basic virtio-mmio support on Arm

From: Julien Grall <julien@xen.org>
To: Oleksandr <olekstysh@gmail.com>
Cc: xen-devel@lists.xenproject.org,
	Julien Grall <julien.grall@arm.com>,
	Ian Jackson <iwj@xenproject.org>, Wei Liu <wl@xen.org>,
	Anthony PERARD <anthony.perard@citrix.com>,
	Stefano Stabellini <sstabellini@kernel.org>,
	Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>,
	Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
Subject: Re: [PATCH V4 23/24] libxl: Introduce basic virtio-mmio support on Arm
Date: Wed, 20 Jan 2021 16:40:23 +0000	[thread overview]
Message-ID: <58c9da23-ef6a-1d33-b2ec-30e3425da2f3@xen.org> (raw)
In-Reply-To: <51d44085-f178-3985-9324-da6494cd9d2e@gmail.com>

Hi Oleksandr,

On 17/01/2021 22:22, Oleksandr wrote:
> On 15.01.21 23:30, Julien Grall wrote:
>> On 12/01/2021 21:52, Oleksandr Tyshchenko wrote:
>>> From: Julien Grall <julien.grall@arm.com>
>> So I am not quite too sure how this new parameter can be used. Could 
>> you expand it?
> The original idea was to set it if we are going to assign virtio 
> device(s) to the guest.
> Being honest, I have a plan to remove this extra parameter. It might not 
> be obvious looking at the current patch, but next patch will show that 
> we can avoid introducing it at all.

Right, so I think we want to avoid introducing the parameter. I have 
suggested in patch #24 a different way to split code introduced by #23 
and #24.

[...]

>>
>>> +#define GUEST_VIRTIO_MMIO_SIZE xen_mk_ullong(0x200)
>>
>> AFAICT, the size of the virtio mmio region should be 0x100. So why is 
>> it 0x200?
> 
> 
> I didn't find the total size requirement for the mmio region in virtio 
> specification v1.1 (the size of control registers is indeed 0x100 and 
> device-specific configuration registers starts at the offset 0x100, 
> however it's size depends on the device and the driver).
> 
> kvmtool uses 0x200 [1], in some Linux device-trees we can see 0x200 [2] 
> (however, device-tree bindings example has 0x100 [3]), so what would be 
> the proper value for Xen code?

Hmm... I missed that fact. I would say we want to use the biggest size 
possible so we can cover most of the devices.

Although, as you pointed out, this may not cover all the devices. So 
maybe we want to allow the user to configure the size via xl.cfg for the 
one not conforming with 0x200.

This could be implemented in the future. Stefano/Ian, what do you think?

>> Most likely, you will want to reserve a range
> 
> it seems yes, good point. BTW, the range is needed for the mmio region 
> as well, correct?

I would reserve 1MB (just for the sake of avoid region size in KB).

For the SPIs, I would consider to reserve 10-20 interrupts. Do you think 
this will cover your use cases?

Cheers,

-- 
Julien Grall