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

Re: [Xen-devel] [PATCH v6 17/31] xen/arm: ITS: Store LPIs allocated and IRQ ID bits per domain



Hi Vijay,

On 07/09/15 07:59, Vijay Kilari wrote:
> On Thu, Sep 3, 2015 at 9:55 PM, Julien Grall <julien.grall@xxxxxxxxxx> wrote:
>> I still don't think that it makes sense to introduce the number of LPIs
>> variable in a patch for vITS. As you describe it, it should be part of
>> an ITS/GICv3 patch.
>>
>> Although, I think you should use the field nr_irq_ids in the gic
>> structure (see patch #10) to avoid problem you currently have with this
>> solution.
>>
>> For instance, gic_is_lpi is relying on the number of ID bits exposed by
>> the hardware but you allocate the IRQ desc for LPIs based on nr_lpis.
>>
> 
> You mean to restrict nr_irq_ids to FIRST_GIC_LPI + 8192 (nr_lpis)
> instead of HW supported value?
> and let gic_is_lpi() always check for Xen supported lpi number instead of hw
> supported value?

Yes for both.

>> It's fine to restrict the value in the GIC compare to hardware.
>>
>> Furthermore this value should be 0 on platform where LPIs is not
>> supported in order to make confusion and introduce possible problem with
>> the code later. I could add that, AFAICT, this new variable (nr_lpis) is
>> not that often within the code.
> 
>  nr_lpis is global variable that we define to tell number of LPIs supported
> by Xen. If platform does not support LPIs, then vgic.nr_lpis is set to 0.

While you set vgic.nr_lpis to 0, you don't clear nr_lpis. So
technically, someone could decide to use it later but it won't report
the correct value....

It was difficult to find the correct place to comment about this given
that you've introduced the variable in an odd place....

> 
>>
>> Lastly, on a previous version (don't remember which one) we said that
>> the user should be able to change the value on the command line. What's
>> your plan for that?
> 
> Yes, it was part of our discussion on chat. I will fix it.

A summarize would have been nice... like a TODO in the code and/or in
the cover letter. Depends when you plan to handle it.

Regards,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.