Hi, Thomas and Marc,
On Wed, 2018-01-17 at 11:01 +0100, Thomas Gleixner wrote:
> On Wed, 17 Jan 2018, Yang, Shunyong wrote:
> >
> > On Wed, 2018-01-17 at 10:33 +0100, Thomas Gleixner wrote:
> > >
> > > And how is that different from:
> > >
> > > >
> > > >
> > > > The nodes under debugfs irq/irqs describes information of every
> > > > single
> > > > irq.
> > > Not at all. It contains the complete hierarchical information of
> > > each
> > > virq.
> > >
> > I think irq_domain_mapping can provide some high-level information
> > in a
> > summary style.
> > For example, we can check all the IRQs connect to a specific irq
> > chip
> > or irq domain.
> You can retrieve the same information from the irq/irqs files. All it
> takes
> is a shell script.
>
> Aside of that with hierarchical irq domains the old debug output is
> just
> useless.
>
Umm...Agree. Need I post a patch to remove it?
Thanks
Shunyong
On Wed, 17 Jan 2018, Yang, Shunyong wrote:
> On Wed, 2018-01-17 at 11:01 +0100, Thomas Gleixner wrote:
> > You can retrieve the same information from the irq/irqs files. All it
> > takes
> > is a shell script.
> >
> > Aside of that with hierarchical irq domains the old debug output is
> > just
> > useless.
> >
> Umm...Agree. Need I post a patch to remove it?
Would be appreciated.
Thanks,
tglx
On 17/01/18 10:20, Yang, Shunyong wrote:
> Hi, Thomas and Marc,
>
> On Wed, 2018-01-17 at 11:01 +0100, Thomas Gleixner wrote:
>> On Wed, 17 Jan 2018, Yang, Shunyong wrote:
>>>
>>> On Wed, 2018-01-17 at 10:33 +0100, Thomas Gleixner wrote:
>>>>
>>>> And how is that different from:
>>>>
>>>>>
>>>>>
>>>>> The nodes under debugfs irq/irqs describes information of every
>>>>> single
>>>>> irq.
>>>> Not at all. It contains the complete hierarchical information of
>>>> each
>>>> virq.
>>>>
>>> I think irq_domain_mapping can provide some high-level information
>>> in a
>>> summary style.
>>> For example, we can check all the IRQs connect to a specific irq
>>> chip
>>> or irq domain.
>> You can retrieve the same information from the irq/irqs files. All it
>> takes
>> is a shell script.
>>
>> Aside of that with hierarchical irq domains the old debug output is
>> just
>> useless.
>>
> Umm...Agree. Need I post a patch to remove it?
I'm on it.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
Hi, Marc
On Wed, 2018-01-17 at 10:43 +0000, Marc Zyngier wrote:
> On 17/01/18 10:20, Yang, Shunyong wrote:
> >
> > Hi, Thomas and Marc,
> >
> > On Wed, 2018-01-17 at 11:01 +0100, Thomas Gleixner wrote:
> > >
> > > On Wed, 17 Jan 2018, Yang, Shunyong wrote:
> > > >
> > > >
> > > > On Wed, 2018-01-17 at 10:33 +0100, Thomas Gleixner wrote:
> > > > >
> > > > >
> > > > > And how is that different from:
> > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > > The nodes under debugfs irq/irqs describes information of
> > > > > > every
> > > > > > single
> > > > > > irq.
> > > > > Not at all. It contains the complete hierarchical information
> > > > > of
> > > > > each
> > > > > virq.
> > > > >
> > > > I think irq_domain_mapping can provide some high-level
> > > > information
> > > > in a
> > > > summary style.
> > > > For example, we can check all the IRQs connect to a specific
> > > > irq
> > > > chip
> > > > or irq domain.
> > > You can retrieve the same information from the irq/irqs files.
> > > All it
> > > takes
> > > is a shell script.
> > >
> > > Aside of that with hierarchical irq domains the old debug output
> > > is
> > > just
> > > useless.
> > >
> > Umm...Agree. Need I post a patch to remove it?
> I'm on it.
>
In addition to the "%p" to "%px" change in?IRQ_DOMAIN_DEBUG you have
posted patch to remove it, my original patch includes several other
changes:
1. change to "%px" in kasprintf() parameters in
function?__irq_domain_alloc_fwnode() to build name which reflects the
real pointer address from caller, this may be useful when reading debug
information.
?
n = kasprintf(GFP_KERNEL, "irqchip@%px", data);
2. other 3 changes from "%p" to "%px" in existing?pr_debug().
Could I submit a V2 patch which depends on your "irqdomain: Kill
CONFIG_IRQ_DOMAIN_DEBUG" patch to handle this??
Following is the link of your patch,
https://patchwork.kernel.org/patch/10169367/?
.
Thanks
Shunyong
On 18/01/18 01:53, Yang, Shunyong wrote:
> Hi, Marc
>
> On Wed, 2018-01-17 at 10:43 +0000, Marc Zyngier wrote:
>> On 17/01/18 10:20, Yang, Shunyong wrote:
>>>
>>> Hi, Thomas and Marc,
>>>
>>> On Wed, 2018-01-17 at 11:01 +0100, Thomas Gleixner wrote:
>>>>
>>>> On Wed, 17 Jan 2018, Yang, Shunyong wrote:
>>>>>
>>>>>
>>>>> On Wed, 2018-01-17 at 10:33 +0100, Thomas Gleixner wrote:
>>>>>>
>>>>>>
>>>>>> And how is that different from:
>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> The nodes under debugfs irq/irqs describes information of
>>>>>>> every
>>>>>>> single
>>>>>>> irq.
>>>>>> Not at all. It contains the complete hierarchical information
>>>>>> of
>>>>>> each
>>>>>> virq.
>>>>>>
>>>>> I think irq_domain_mapping can provide some high-level
>>>>> information
>>>>> in a
>>>>> summary style.
>>>>> For example, we can check all the IRQs connect to a specific
>>>>> irq
>>>>> chip
>>>>> or irq domain.
>>>> You can retrieve the same information from the irq/irqs files.
>>>> All it
>>>> takes
>>>> is a shell script.
>>>>
>>>> Aside of that with hierarchical irq domains the old debug output
>>>> is
>>>> just
>>>> useless.
>>>>
>>> Umm...Agree. Need I post a patch to remove it?
>> I'm on it.
>>
> In addition to the "%p" to "%px" change in?IRQ_DOMAIN_DEBUG you have
> posted patch to remove it, my original patch includes several other
> changes:
> 1. change to "%px" in kasprintf() parameters in
> function?__irq_domain_alloc_fwnode() to build name which reflects the
> real pointer address from caller, this may be useful when reading debug
> information.
> ?
> n = kasprintf(GFP_KERNEL, "irqchip@%px", data);
Have you investigated whether %pK would work in a debug context?
>
> 2. other 3 changes from "%p" to "%px" in existing?pr_debug().
>
> Could I submit a V2 patch which depends on your "irqdomain: Kill
> CONFIG_IRQ_DOMAIN_DEBUG" patch to handle this??
> Following is the link of your patch,
> https://patchwork.kernel.org/patch/10169367/?
Sure. If you think something is missing, feel free to send an additional
patch.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
Hi, Marc
On Thu, 2018-01-18 at 08:54 +0000, Marc Zyngier wrote:
> On 18/01/18 01:53, Yang, Shunyong wrote:
> >
> > Hi, Marc
> >
> > On Wed, 2018-01-17 at 10:43 +0000, Marc Zyngier wrote:
> > >
> > > On 17/01/18 10:20, Yang, Shunyong wrote:
> > > >
> > > >
> > > > Hi, Thomas and Marc,
> > > >
> > > > On Wed, 2018-01-17 at 11:01 +0100, Thomas Gleixner wrote:
> > > > >
> > > > >
> > > > > On Wed, 17 Jan 2018, Yang, Shunyong wrote:
> > > > > >
> > > > > >
> > > > > >
> > > > > > On Wed, 2018-01-17 at 10:33 +0100, Thomas Gleixner wrote:
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > And how is that different from:
> > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > The nodes under debugfs irq/irqs describes information
> > > > > > > > of
> > > > > > > > every
> > > > > > > > single
> > > > > > > > irq.
> > > > > > > Not at all. It contains the complete hierarchical
> > > > > > > information
> > > > > > > of
> > > > > > > each
> > > > > > > virq.
> > > > > > >
> > > > > > I think irq_domain_mapping can provide some high-level
> > > > > > information
> > > > > > in a
> > > > > > summary style.
> > > > > > For example, we can check all the IRQs connect to a
> > > > > > specific
> > > > > > irq
> > > > > > chip
> > > > > > or irq domain.
> > > > > You can retrieve the same information from the irq/irqs
> > > > > files.
> > > > > All it
> > > > > takes
> > > > > is a shell script.
> > > > >
> > > > > Aside of that with hierarchical irq domains the old debug
> > > > > output
> > > > > is
> > > > > just
> > > > > useless.
> > > > >
> > > > Umm...Agree. Need I post a patch to remove it?
> > > I'm on it.
> > >
> > In addition to the "%p" to "%px" change in?IRQ_DOMAIN_DEBUG you
> > have
> > posted patch to remove it, my original patch includes several other
> > changes:
> > 1. change to "%px" in kasprintf() parameters in
> > function?__irq_domain_alloc_fwnode() to build name which reflects
> > the
> > real pointer address from caller, this may be useful when reading
> > debug
> > information.
> > ?
> > n = kasprintf(GFP_KERNEL, "irqchip@%px", data);
> Have you investigated whether %pK would work in a debug context?
Although build name with physical address makes it straightforward when
working with tables like MADT. I agree with you about the security
concern. As it's just a name, hashed address is OK.
But it brings new issues. Currently, component like GIC calls this
function with virtual address and ITS with physical address. Is there
possibility that GIC/ITS name is different in each boot? Should we
change GIC/ITS fw handle allocate to name/name-id style?
Thanks
Shunyong?