Re: [PATCH v3 01/10] kvm: arm64: vgic: Introduce vgic_check_iorange

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

	M.

-- 
Without deviation from the norm, progress is not possible.
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm



[Index of Archives]     [Linux KVM]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux