On Fri, Oct 01, 2021 at 02:12:17PM +0100, Marc Zyngier wrote:
> On Thu, 30 Sep 2021 22:19:16 +0100,
> Ricardo Koller <ricarkol@xxxxxxxxxx> wrote:
> >
> > On Thu, Sep 30, 2021 at 09:02:12AM +0200, Eric Auger wrote:
> > > Hi,
> > >
> > > On 9/29/21 11:17 PM, Ricardo Koller wrote:
> > > > On Wed, Sep 29, 2021 at 06:29:21PM +0200, Eric Auger wrote:
> > > >> Hi Ricardo,
> > > >>
> > > >> On 9/28/21 8:47 PM, Ricardo Koller wrote:
> > > >>> Add the new vgic_check_iorange helper that checks that an iorange is
> > > >>> sane: the start address and size have valid alignments, the range is
> > > >>> within the addressable PA range, start+size doesn't overflow, and the
> > > >>> start wasn't already defined.
> > > >>>
> > > >>> No functional change.
> > > >>>
> > > >>> Signed-off-by: Ricardo Koller <ricarkol@xxxxxxxxxx>
> > > >>> ---
> > > >>> arch/arm64/kvm/vgic/vgic-kvm-device.c | 22 ++++++++++++++++++++++
> > > >>> arch/arm64/kvm/vgic/vgic.h | 4 ++++
> > > >>> 2 files changed, 26 insertions(+)
> > > >>>
> > > >>> diff --git a/arch/arm64/kvm/vgic/vgic-kvm-device.c b/arch/arm64/kvm/vgic/vgic-kvm-device.c
> > > >>> index 7740995de982..f714aded67b2 100644
> > > >>> --- a/arch/arm64/kvm/vgic/vgic-kvm-device.c
> > > >>> +++ b/arch/arm64/kvm/vgic/vgic-kvm-device.c
> > > >>> @@ -29,6 +29,28 @@ int vgic_check_ioaddr(struct kvm *kvm, phys_addr_t *ioaddr,
> > > >>> return 0;
> > > >>> }
> > > >>>
> > > >>> +int vgic_check_iorange(struct kvm *kvm, phys_addr_t *ioaddr,
> > > >>> + phys_addr_t addr, phys_addr_t alignment,
> > > >>> + phys_addr_t size)
> > > >>> +{
> > > >>> + int ret;
> > > >>> +
> > > >>> + ret = vgic_check_ioaddr(kvm, ioaddr, addr, alignment);
> > > >> nit: not related to this patch but I am just wondering why we are
> > > >> passing phys_addr_t *ioaddr downto vgic_check_ioaddr and thus to
> > > >>
> > > >> vgic_check_iorange()? This must be a leftover of some old code?
> > > >>
> > > > It's used to check that the base of a region is not already set.
> > > > kvm_vgic_addr() uses it to make that check;
> > > > vgic_v3_alloc_redist_region() does not:
> > > >
> > > > rdreg->base = VGIC_ADDR_UNDEF; // so the "not already defined" check passes
> > > > ret = vgic_check_ioaddr(kvm, &rdreg->base, base, SZ_64K);
> > > Yes but I meant why a pointer?
> >
> > I can't think of any good reason. It must be some leftover as you said.
>
> It definitely is. Please have a patch to fix that. Also, it doesn't
> look like vgic_check_ioaddr() has any other user at the end of the
> series. Worth getting rid of.
ACK fixing that and getting rid of vgic_check_ioaddr().
Thanks,
Ricardo
>
> M.
>
> --
> Without deviation from the norm, progress is not possible.
