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

[Oleksandr <olekstysh@xxxxxxxxx>]

On 20.01.21 18:40, Julien Grall wrote:
> Hi Oleksandr,
Hi Julien


Sorry for the late response.


> 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@xxxxxxx>
> > > 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.
Got it. Will take it into the account for the next version.


> [...]
>
> > > > +#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?
I see that Stefano has already agreed on that, so let's leave 0x200 for now.


> > > 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?
Yes, I think it would be enough for now.


> Cheers,
--
Regards,

Oleksandr Tyshchenko