From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vijay Kilari Subject: Re: [PATCH v4 06/17] xen/arm: ITS: Add virtual ITS driver Date: Sat, 11 Jul 2015 20:18:36 +0530 Message-ID: References: <1436514172-3263-1-git-send-email-vijay.kilari@gmail.com> <1436514172-3263-7-git-send-email-vijay.kilari@gmail.com> <1436537742.10074.50.camel@xen.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1436537742.10074.50.camel@xen.org> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: Stefano Stabellini , Prasun Kapoor , Vijaya Kumar K , Tim Deegan , "xen-devel@lists.xen.org" , Julien Grall , Stefano Stabellini , manish.jaggi@caviumnetworks.com List-Id: xen-devel@lists.xenproject.org On Fri, Jul 10, 2015 at 7:45 PM, Ian Campbell wrote: > On Fri, 2015-07-10 at 13:12 +0530, vijay.kilari@gmail.com wrote: >> +static int vits_entry(struct domain *d, paddr_t entry, void *addr, >> + uint32_t size, bool_t set) >> +{ >> [...] >> +} >> + >> +/* ITS device table helper functions */ >> +static int vits_vdevice_entry(struct domain *d, uint32_t dev_id, >> + struct vdevice_table *entry, bool_t set) >> +{ >> + uint64_t offset; >> + paddr_t dt_entry; >> + >> + BUILD_BUG_ON(sizeof(struct vdevice_table) != 16); >> + >> + offset = dev_id * sizeof(struct vdevice_table); >> + if ( offset > d->arch.vits->dt_size ) >> + { >> + dprintk(XENLOG_G_ERR, >> + "%pv: vITS: Out of range offset %ld id 0x%x size %ld\n", >> + current, offset, dev_id, d->arch.vits->dt_size); >> + return -EINVAL; >> + } >> + >> + dt_entry = d->arch.vits->dt_ipa + offset; >> + >> + return vits_entry(d, dt_entry, (void *)entry, >> + sizeof(struct vdevice_table), > > Please drop the (void *) cast here, you can pass a "foo *" to a "void *" > without one. > > It took me a little while to work out why this was void * before I > realised that vits_entry was a generic helper used for different types > of table. "vits_access_guest_table" to make it clear what it is doing. This is also used in later patches read virtual ITS command and also property pending table. I prefer to move it to some generic file like guestcopy.c/p2m.c? and should be named as copy_{from|to}guest_gfn()?