From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v4 12/17] xen/arm: ITS: Initialize LPI irq descriptors and route Date: Thu, 16 Jul 2015 10:06:58 +0200 Message-ID: <55A76622.4070200@citrix.com> References: <1436514172-3263-1-git-send-email-vijay.kilari@gmail.com> <1436514172-3263-13-git-send-email-vijay.kilari@gmail.com> <55A6A2D3.5060903@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <55A6A2D3.5060903@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: vijay.kilari@gmail.com, Ian.Campbell@citrix.com, stefano.stabellini@eu.citrix.com, stefano.stabellini@citrix.com, tim@xen.org, xen-devel@lists.xen.org Cc: Prasun.Kapoor@caviumnetworks.com, Vijaya Kumar K , manish.jaggi@caviumnetworks.com List-Id: xen-devel@lists.xenproject.org Hi Vijay, On 15/07/2015 20:13, Julien Grall wrote: >> +uint8_t vgic_its_get_priority(struct vcpu *v, uint32_t pid) >> +{ >> + uint8_t priority; >> + >> + priority = readb_relaxed(v->domain->arch.vits->prop_page + pid); > > Why do you use readb_relaxed here? This should only be used for Device > MMIO. > > Although, you need to ensure that the value will be correctly > synchronize if another CPU is writing in prop_page which is protected by > prop_lock. I though a bit more during the night about this function. On patch #11, where you allocate prop_page, you allow to have a smaller table than the number of LPIs. If the pid is too high, even though valid, you may read Xen memory or even crash Xen. Although, what does mean pid? Should not it be vlpi? Regards, -- Julien Grall