linux-kernel - Re: [PATCH v5 10/11] powerpc/pseries/iommu: Make use of DDW for indirect mapping

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]

Message-ID: <75c84c0b-46b3-2600-c186-257aec05c645@ozlabs.ru>
Date:   Fri, 23 Jul 2021 15:34:25 +1000
From:   Alexey Kardashevskiy <aik@...abs.ru>
To:     Frederic Barrat <fbarrat@...ux.ibm.com>,
        Leonardo Bras <leobras.c@...il.com>,
        Michael Ellerman <mpe@...erman.id.au>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Paul Mackerras <paulus@...ba.org>,
        David Gibson <david@...son.dropbear.id.au>,
        kernel test robot <lkp@...el.com>,
        Nicolin Chen <nicoleotsuka@...il.com>
Cc:     linuxppc-dev@...ts.ozlabs.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 10/11] powerpc/pseries/iommu: Make use of DDW for
 indirect mapping



On 22/07/2021 01:04, Frederic Barrat wrote:
> 
> 
> On 21/07/2021 05:32, Alexey Kardashevskiy wrote:
>>>> +        struct iommu_table *newtbl;
>>>> +        int i;
>>>> +
>>>> +        for (i = 0; i < ARRAY_SIZE(pci->phb->mem_resources); i++) {
>>>> +            const unsigned long mask = IORESOURCE_MEM_64 | 
>>>> IORESOURCE_MEM;
>>>> +
>>>> +            /* Look for MMIO32 */
>>>> +            if ((pci->phb->mem_resources[i].flags & mask) == 
>>>> IORESOURCE_MEM)
>>>> +                break;
>>>> +        }
>>>> +
>>>> +        if (i == ARRAY_SIZE(pci->phb->mem_resources))
>>>> +            goto out_del_list;
>>>
>>>
>>> So we exit and do nothing if there's no MMIO32 bar?
>>> Isn't the intent just to figure out the MMIO32 area to reserve it 
>>> when init'ing the table? In which case we could default to 0,0
>>>
>>> I'm actually not clear why we are reserving this area on pseries.
>>
>>
>>
>> If we do not reserve it, then the iommu code will allocate DMA pages 
>> from there and these addresses are MMIO32 from the kernel pov at 
>> least. I saw crashes when (I think) a device tried DMAing to the top 
>> 2GB of the bus space which happened to be a some other device's BAR.
> 
> 
> hmmm... then figuring out the correct range needs more work. We could 
> have more than one MMIO32 bar. And they don't have to be adjacent. 

They all have to be within the MMIO32 window of a PHB and we reserve the 
entire window here.

> I 
> don't see that we are reserving any range on the initial table though 
> (on pseries).
True, we did not need to, as the hypervisor always took care of DMA and 
MMIO32 regions to not overlap.

And in this series we do not (strictly speaking) need this either as 
phyp never allocates more than one window dynamically and that only 
window is always the second one starting from 0x800.0000.0000.0000. It 
is probably my mistake that KVM allows a new window to start from 0 - 
PAPR did not prohibit this explicitly.

And for the KVM case, we do not need to remove the default window as KVM 
can pretty much always allocate as many TCE as the VM wants. But we 
still allow removing the default window and creating a huge one instead 
at 0x0 as this way we can allow 1:1 for every single PCI device even if 
it only allows 48 (or similar but less than 64bit) DMA. Hope this makes 
sense. Thanks,


-- 
Alexey