From: Alexey Kardashevskiy <aik@ozlabs•ru>
To: Michael Ellerman <mpe@ellerman•id.au>,
Nishanth Aravamudan <nacc@linux•vnet.ibm.com>
Cc: Hari Bathini <hbathini@linux•vnet.ibm.com>,
Gavin Shan <gwshan@linux•vnet.ibm.com>,
Ben Herrenschmidt <benh@kernel•crashing.org>,
Paul Mackerras <paulus@samba•org>,
David Gibson <david@gibson•dropbear.id.au>,
Wei Yang <weiyang@linux•vnet.ibm.com>,
linuxppc-dev@lists•ozlabs.org
Subject: Re: [PATCH v2] powerpc/powernv/pci-ioda: fix 32-bit TCE table init in kdump kernel
Date: Fri, 4 Sep 2015 13:41:20 +1000 [thread overview]
Message-ID: <55E912E0.1000602@ozlabs.ru> (raw)
In-Reply-To: <1441274333.26379.4.camel@ellerman.id.au>
On 09/03/2015 07:58 PM, Michael Ellerman wrote:
> On Wed, 2015-09-02 at 08:39 -0700, Nishanth Aravamudan wrote:
>> On 02.09.2015 [19:00:31 +1000], Alexey Kardashevskiy wrote:
>>> On 09/02/2015 11:11 AM, Nishanth Aravamudan wrote:
>>>> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
>>>> index 85cbc96eff6c..0d7967e31169 100644
>>>> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
>>>> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
>>>> @@ -2077,10 +2077,17 @@ static long pnv_pci_ioda2_setup_default_config(struct pnv_ioda_pe *pe)
>>>> {
>>>> struct iommu_table *tbl = NULL;
>>>> long rc;
>>>> + /*
>>>> + * In memory constrained environments, e.g. kdump kernel, the
>>>> + * DMA window can be larger than available memory, which will
>>>> + * cause errors later.
>>>> + */
>>>> + __u64 window_size =
>>>
>>> I asked for "const __u64" ;)
>>
>> I knew I'd forget something!
>
> Nish! In future please send a reply with the above comment, and then the v2 as
> a separate mail, otherwise I have to manually edit out your comment when
> applying.
>
>> When attempting to kdump with the 4.2 kernel, we see for each PCI
>> device:
>>
>> pci 0003:01 : [PE# 000] Assign DMA32 space
>> pci 0003:01 : [PE# 000] Setting up 32-bit TCE table at 0..80000000
>> pci 0003:01 : [PE# 000] Failed to create 32-bit TCE table, err -22
>> PCI: Domain 0004 has 8 available 32-bit DMA segments
>> PCI: 4 PE# for a total weight of 70
>> pci 0004:01 : [PE# 002] Assign DMA32 space
>> pci 0004:01 : [PE# 002] Setting up 32-bit TCE table at 0..80000000
>> pci 0004:01 : [PE# 002] Failed to create 32-bit TCE table, err -22
>> pci 0004:0d : [PE# 005] Assign DMA32 space
>> pci 0004:0d : [PE# 005] Setting up 32-bit TCE table at 0..80000000
>> pci 0004:0d : [PE# 005] Failed to create 32-bit TCE table, err -22
>> pci 0004:0e : [PE# 006] Assign DMA32 space
>> pci 0004:0e : [PE# 006] Setting up 32-bit TCE table at 0..80000000
>> pci 0004:0e : [PE# 006] Failed to create 32-bit TCE table, err -22
>> pci 0004:10 : [PE# 008] Assign DMA32 space
>> pci 0004:10 : [PE# 008] Setting up 32-bit TCE table at 0..80000000
>> pci 0004:10 : [PE# 008] Failed to create 32-bit TCE table, err -22
>>
>> and eventually the kdump kernel fails to boot as none of the PCI devices
>> (including the disk controller) are successfully initialized.
>>
>> The EINVAL response is because the DMA window (the 2GB base window) is
>> larger than the kdump kernel's reserved memory (crashkernel=, in this
>> case specified to be 1024M). The check in question,
>>
>> if ((window_size > memory_hotplug_max()) || !is_power_of_2(window_size))
>>
>> is a valid sanity check for pnv_pci_ioda2_table_alloc_pages(), so adjust
>> the caller to pass in a smaller window size if our maximum memory value
>> is smaller than the DMA window.
>>
>> After this change, the PCI devices successfully set up the 32-bit TCE
>> table and kdump succeeds.
>>
>> The problem was seen on a Firestone machine originally.
>>
>> Fixes: aca6913f5551 ("powerpc/powernv/ioda2: Introduce helpers to allocate TCE pages")
>
> That went into 4.2, so should this have a:
>
> Cc: stable@vger•kernel.org # 4.2
>
> ?
>
>> diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
>> index 85cbc96eff6c..e51aff01a218 100644
>> --- a/arch/powerpc/platforms/powernv/pci-ioda.c
>> +++ b/arch/powerpc/platforms/powernv/pci-ioda.c
>> @@ -2077,10 +2077,17 @@ static long pnv_pci_ioda2_setup_default_config(struct pnv_ioda_pe *pe)
>> {
>> struct iommu_table *tbl = NULL;
>> long rc;
>> + /*
>> + * In memory constrained environments, e.g. kdump kernel, the
>> + * DMA window can be larger than available memory, which will
>> + * cause errors later.
>> + */
>> + const __u64 window_size =
>
> Why is this using __u64 and not u64, it's not exported to userspace.
>
> It looks like pnv_pci_ioda2_create_table() uses __u64, but there's no reason
> for that AFAICS either. And yes I did commit it so it's my fault :)
There is VFIO_IOMMU_SPAPR_TCE_CREATE ioctl which receives
vfio_iommu_spapr_tce_create struct from the user space and there is "__u64
window_size" which is passed across:
tce_iommu_create_window()
tce_iommu_create_table()
pnv_pci_ioda2_create_table (via table_group->ops->create_table())
At what point should __u64 have become u64?
Thanks.
>
> I'll fix this up to use u64.
>
> cheers
>
>
>
--
Alexey
next prev parent reply other threads:[~2015-09-04 3:41 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-09-02 1:11 [PATCH] powerpc/powernv/pci-ioda: fix 32-bit TCE table init in kdump kernel Nishanth Aravamudan
2015-09-02 9:00 ` Alexey Kardashevskiy
2015-09-02 15:39 ` [PATCH v2] " Nishanth Aravamudan
2015-09-03 9:58 ` Michael Ellerman
2015-09-03 15:59 ` Nishanth Aravamudan
2015-09-04 9:57 ` Michael Ellerman
2015-09-04 3:41 ` Alexey Kardashevskiy [this message]
2015-09-04 10:03 ` Michael Ellerman
2015-09-08 12:05 ` [v2] " Michael Ellerman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=55E912E0.1000602@ozlabs.ru \
--to=aik@ozlabs$(echo .)ru \
--cc=benh@kernel$(echo .)crashing.org \
--cc=david@gibson$(echo .)dropbear.id.au \
--cc=gwshan@linux$(echo .)vnet.ibm.com \
--cc=hbathini@linux$(echo .)vnet.ibm.com \
--cc=linuxppc-dev@lists$(echo .)ozlabs.org \
--cc=mpe@ellerman$(echo .)id.au \
--cc=nacc@linux$(echo .)vnet.ibm.com \
--cc=paulus@samba$(echo .)org \
--cc=weiyang@linux$(echo .)vnet.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox