Re: [PATCH 04/12] swiotlb-xen: ensure to issue well-formed XENMEM_exchange requests

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Tue, 14 Sep 2021, Jan Beulich wrote:
> On 13.09.2021 22:31, Stefano Stabellini wrote:
> > On Mon, 13 Sep 2021, Jan Beulich wrote:
> >> On 11.09.2021 01:14, Stefano Stabellini wrote:
> >>> On Tue, 7 Sep 2021, Jan Beulich wrote:
> >>>> While the hypervisor hasn't been enforcing this, we would still better
> >>>> avoid issuing requests with GFNs not aligned to the requested order.
> >>>>
> >>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> >>>> ---
> >>>> I wonder how useful it is to include the alignment in the panic()
> >>>> message.
> >>>
> >>> Not very useful given that it is static. I don't mind either way but you
> >>> can go ahead and remove it if you prefer (and it would make the line
> >>> shorter.)
> >>>
> >>>
> >>>> I further wonder how useful it is to wrap "bytes" in
> >>>> PAGE_ALIGN(), when it is a multiple of a segment's size anyway (or at
> >>>> least was supposed to be, prior to "swiotlb-xen: maintain slab count
> >>>> properly").
> >>>
> >>> This one I would keep, to make sure to print out the same amount passed
> >>> to memblock_alloc.
> >>
> >> Oh - if I was to drop it from the printk(), I would have been meaning to
> >> also drop it there. If it's useless, then it's useless everywhere.
> > 
> > That's fine too
> 
> Thanks, I'll see about dropping that then.
> 
> Another Arm-related question has occurred to me: Do you actually
> mind the higher-than-necessary alignment there? If so, a per-arch
> definition of the needed alignment would need introducing. Maybe
> that could default to PAGE_SIZE, allowing Arm and alike to get away
> without explicitly specifying a value ...

Certainly a patch like that could be good. Given that it is only one
allocation I was assuming that the higher-than-necessary alignment
wouldn't be a problem worth addressing (and I cannot completely rule out
that one day we might have to use XENMEM_exchange on ARM too).