From mboxrd@z Thu Jan  1 00:00:00 1970
From: Vijay Kilari <vijay.kilari@gmail.com>
Subject: Re: [PATCH v4 11/17] xen/arm: ITS: Add GICR register
	emulation
Date: Thu, 16 Jul 2015 19:45:28 +0530
Message-ID: <CALicx6vp0gjzE5vy5R_i8FqHGTFEq+kydA=e4L+bxMYtpw2rQg@mail.gmail.com>
References: <1436514172-3263-1-git-send-email-vijay.kilari@gmail.com>
	<1436514172-3263-12-git-send-email-vijay.kilari@gmail.com>
	<55A69925.9070408@citrix.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <55A69925.9070408@citrix.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Julien Grall <julien.grall@citrix.com>
Cc: Ian Campbell <Ian.Campbell@citrix.com>, Stefano Stabellini <stefano.stabellini@eu.citrix.com>, Prasun Kapoor <Prasun.Kapoor@caviumnetworks.com>, Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>, Tim Deegan <tim@xen.org>, "xen-devel@lists.xen.org" <xen-devel@lists.xen.org>, Stefano Stabellini <stefano.stabellini@citrix.com>, manish.jaggi@caviumnetworks.com
List-Id: xen-devel@lists.xenproject.org

On Wed, Jul 15, 2015 at 11:02 PM, Julien Grall <julien.grall@citrix.com> wrote:
> Hi Vijay,
>
> On 10/07/2015 09:42, vijay.kilari@gmail.com wrote:
>>
>> From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
>>
>> Emulate LPI related changes to GICR registers
[..]
>
>> +    p = irq_to_pending(v, vlpi);
>> +
>> +    set_bit(GIC_IRQ_GUEST_ENABLED, &p->status);
>> +
>> +    spin_lock_irqsave(&v->arch.vgic.lock, flags);
>> +
>> +    /*XXX: raise on right vcpu */
>
>
> /* XXX: ... */
>
> I guess you added this comment because I mentioned possible locking problem
> here?

   I meant to find out Collection (VCPU) for this vlpi and inject on that VCPU

>
> If so, did you think how this would affect the vLPI handling to not do the
> correct locking for Xen 4.6?
>
> Note for the others: AFAICT the locking of the pending_irq structure is done
> using the lock of the vCPU where the IRQ is routed. Stefano, please confirm
> if it's true.
>
>> +    if ( !list_empty(&p->inflight) &&
>> +         !test_bit(GIC_IRQ_GUEST_VISIBLE, &p->status) )
>> +        gic_raise_guest_irq(v, irq_to_virq(p->desc), p->priority);
>> +
>> +    spin_unlock_irqrestore(&v->arch.vgic.lock, flags);
>> +}
>> +

[...]

>
>> +    {
>> +        DPRINTK("%pv: vITS: LPI Table read offset 0x%x\n", v, offset);
>> +        spin_lock(&v->domain->arch.vits->prop_lock);
>> +        *r = *((u8*)v->domain->arch.vits->prop_page + offset);
>
>
> You didn't answer my question on v3... I.e "what about other access? a
> 64/32/16 bits access are valid and will return the wrong value."

  One byte represents one LPI in the configuration table. This table being in
memory, guest can access 16/32/64. I will make a check on access size and
handle accordingly.

>> +
>> +    spin_lock(&v->domain->arch.vits->prop_lock);
>
>
> I don't understand why you need to take the prop_lock here. You only use
> them in the handler which you don't have yet registered.
>
    lock is taken as property table can be changed. However as I am
not handling change in property table in this series, It can be dropped.

>> +    if ( id_bits > gic_nr_id_bits() )
>> +        id_bits = gic_nr_id_bits();
>
>
> What happen if the property configuration table is smaller?

Spec does not say anything about it