linux-kernel - Re: [RFC PATCH v0.2] PCI: Add support for tango PCIe host bridge

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 27 Mar 2017 22:07:14 +0100
From:   Marc Zyngier <marc.zyngier@....com>
To:     Mason <slash.tmp@...e.fr>
Cc:     Bjorn Helgaas <helgaas@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Robin Murphy <robin.murphy@....com>,
        Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
        Liviu Dudau <liviu.dudau@....com>,
        David Laight <david.laight@...lab.com>,
        linux-pci <linux-pci@...r.kernel.org>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        Thibaud Cornic <thibaud_cornic@...madesigns.com>,
        Phuong Nguyen <phuong_nguyen@...madesigns.com>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH v0.2] PCI: Add support for tango PCIe host bridge

On Mon, Mar 27 2017 at 08:44:08 PM, Mason <slash.tmp@...e.fr> wrote:
> On 27/03/2017 19:09, Marc Zyngier wrote:
>
>> On 27/03/17 16:53, Mason wrote:
>> 
>>> I have one remaining issue with bitmaps.
>>>
>>> My HW regs are 32b. How do I grab e.g. bits 96-127?
>>> All I can think of is
>>>   u32 val = ((u32 *)bitmap)[3];
>>> 
>>> Is this acceptable?
>> 
>> No.
>> 
>>> mrutland mentioned bitmap_to_u32array() but IIUC it is used to
>>> copy an entire bitmap.
>> 
>> The real question is "Why do you need such a thing?".
>
> You told me to use an in-memory version of the "unmasked"
> bitmap, yes? In that case, I need to be able to grab
> a piece of said bitmap, to update the corresponding piece
> of the HW bitmap.
>
> For example, assume the first 3 MSIs are unmasked,
> in other words, unmasked = 0x7
>
> A new user comes along and wants to assign an MSI.
> Scan "unmasked", found bit at pos 3.
> Update "unmasked" to 0xf.
> At some point, I need to write 0xf to some HW reg.
> So I need to grab a piece of "unmasked" (bits 0-31 in my example)
>
>   pos = find_first_zero_bit(unmasked, COUNT);
>   __set_bit(pos, unmasked);
>   int reg_index = pos / 32;
>   u32 val = ((u32 *)unmasked)[reg_index];
>   writel_relaxed(val, pcie->enabled + reg_index * 4);
>
> Or did I miss something in your suggestion?

I don't know, I'm sightly taken aback by your question. Completely
puzzled, actually. "Read Modify Write" is a fairly obvious construct.

     val = readl_relaxed(pcie->enabled + reg_index);
     writel_relaxed(val | BIT(pos % 32), pcie->enabled + reg_index);

I never realized this could be such a novel concept. Replace pos with
hwirq, add a spinlock, and that's your irq_unmask.

My suggestion was to use a bitmap in order not to perform extra MMIO
accesses on the fast path. It doesn't mean that you cannot read from the
register under any circumstances. It just means that you don't do it
when there are more efficient ways to do it.

	M.
-- 
Jazz is not dead. It just smells funny.