Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 14 Sep 2021 18:54:16 -0700 (PDT)
From:   Stefano Stabellini <sstabellini@kernel.org>
To:     Stefano Stabellini <sstabellini@kernel.org>
cc:     Jan Beulich <jbeulich@suse.com>, Juergen Gross <jgross@suse.com>,
        Boris Ostrovsky <boris.ostrovsky@oracle.com>,
        lkml <linux-kernel@vger.kernel.org>,
        "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>
Subject: Re: [PATCH 04/12] swiotlb-xen: ensure to issue well-formed
 XENMEM_exchange requests
In-Reply-To: <alpine.DEB.2.21.2109141817150.21985@sstabellini-ThinkPad-T480s>
Message-ID: <alpine.DEB.2.21.2109141853510.21985@sstabellini-ThinkPad-T480s>
References: <588b3e6d-2682-160c-468e-44ca4867a570@suse.com> <397bf325-f81e-e104-6142-e8c9c4955475@suse.com> <alpine.DEB.2.21.2109101613130.10523@sstabellini-ThinkPad-T480s> <d868bbcc-e800-ed30-3524-a30a5feb7e5a@suse.com> <alpine.DEB.2.21.2109131328130.10523@sstabellini-ThinkPad-T480s>
 <9819a6e9-93d5-e62a-7b4a-ffc2ecd996dc@suse.com> <alpine.DEB.2.21.2109141817150.21985@sstabellini-ThinkPad-T480s>
User-Agent: Alpine 2.21 (DEB 202 2017-01-01)
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Precedence: bulk

On Tue, 14 Sep 2021, Stefano Stabellini wrote:
> 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@suse.com>
> > >>>> ---
> > >>>> 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).

Also this code is currently #ifdef CONFIG_X86