Re: [Linux-nvdimm] [PATCH 04/21] nd: create an 'nd_bus' from an 'nfit_desc'
On Tue, 2015-04-28 at 10:47 -0600, Toshi Kani wrote:
> On Wed, 2015-04-22 at 13:00 -0700, Dan Williams wrote:
> > On Wed, Apr 22, 2015 at 12:38 PM, Toshi Kani wrote:
> > > On Wed, 2015-04-22 at 12:28 -0700, Dan Williams wrote:
> > >> On Wed, Apr 22, 2015 at 11:23 AM, Toshi Kani wrote:
> > >> > On Wed, 2015-04-22 at 11:20 -0700, Dan Williams wrote:
> > >> >> On Wed, Apr 22, 2015 at 11:00 AM, Linda Knippers
> > >> >> wrote:
> > >> >> Wait, point of clarification, DCRs (dimm-control-regions) have RFICs,
> > >> >> not MEMDEVs (memory-device-to-spa-mapping). Toshi's original report
> > >> >> was that an NFIT with a SPA+MEMDEV was failing to enable a PMEM
> > >> >> device. That specific problem can be fixed by either deleting the
> > >> >> MEMDEV, or adding a DCR.
> > >> >
> > >> > By a DCR, do you mean a DCR structure or SPA with Control Region GUID?
> > >>
> > >> Hmm, I meant a DCR as defined below. I agree you would not need a
> > >> "SPA-DCR".
> > >>
> > >> > Adding a DCR structure does not solve this issue since it requires SPA
> > >> > with Control Region GUID, which battery-backed DIMMs do not have.
> > >>
> > >> I would not go that far, half of a DCR entry is relevant for any
> > >> NVDIMM, and half is only relevant if a DIMM offers BLK access:
> > >>
> > >> struct acpi_nfit_dcr {
> > >> u16 type;
> > >> u16 length;
> > >> u16 dcr_index;
> > >> u16 vendor_id;
> > >> u16 device_id;
> > >> u16 revision_id;
> > >> u16 sub_vendor_id;
> > >> u16 sub_device_id;
> > >> u16 sub_revision_id;
> > >> u8 reserved[6];
> > >> u32 serial_number;
> > >> u16 fic;
> > >> < BLK relevant fields start here <
> > >> u16 num_bcw;
> > >> u64 bcw_size;
> > >> u64 cmd_offset;
> > >> u64 cmd_size;
> > >> u64 status_offset;
> > >> u64 status_size;
> > >> u16 flags;
> > >> u8 reserved2[6];
> > >> };
> > >
> > > Yes, we do have a DCR entry. But we do not have a SPA-DCR.
> >
> > Got it. will fix.
>
> Attached is an example implementation of the NFIT table with 2
> battery-backed NVDIMM cards, which I have used for testing. I hope this
> provides a good example of an NFIT table with SPA(PMEM), MEMDEV and DCR
> entries, which allows optional _DSMs for battery-backed NVDIMMs as
> necessary.
>
> HP is also defining _DSM method for battery-backed NVDIMMs, and will
> share the spec when it is ready.
Sorry, using ".txt" extension to a Linux text file caused my mailer to
perform some unnecessary conversion... Attached is the same file without
".txt" this time.
-Toshi
hp_nfit
Description: Binary data
Re: [Linux-nvdimm] [PATCH 04/21] nd: create an 'nd_bus' from an 'nfit_desc'
On Wed, 2015-04-22 at 13:00 -0700, Dan Williams wrote:
> On Wed, Apr 22, 2015 at 12:38 PM, Toshi Kani wrote:
> > On Wed, 2015-04-22 at 12:28 -0700, Dan Williams wrote:
> >> On Wed, Apr 22, 2015 at 11:23 AM, Toshi Kani wrote:
> >> > On Wed, 2015-04-22 at 11:20 -0700, Dan Williams wrote:
> >> >> On Wed, Apr 22, 2015 at 11:00 AM, Linda Knippers
> >> >> wrote:
> >> >> Wait, point of clarification, DCRs (dimm-control-regions) have RFICs,
> >> >> not MEMDEVs (memory-device-to-spa-mapping). Toshi's original report
> >> >> was that an NFIT with a SPA+MEMDEV was failing to enable a PMEM
> >> >> device. That specific problem can be fixed by either deleting the
> >> >> MEMDEV, or adding a DCR.
> >> >
> >> > By a DCR, do you mean a DCR structure or SPA with Control Region GUID?
> >>
> >> Hmm, I meant a DCR as defined below. I agree you would not need a
> >> "SPA-DCR".
> >>
> >> > Adding a DCR structure does not solve this issue since it requires SPA
> >> > with Control Region GUID, which battery-backed DIMMs do not have.
> >>
> >> I would not go that far, half of a DCR entry is relevant for any
> >> NVDIMM, and half is only relevant if a DIMM offers BLK access:
> >>
> >> struct acpi_nfit_dcr {
> >> u16 type;
> >> u16 length;
> >> u16 dcr_index;
> >> u16 vendor_id;
> >> u16 device_id;
> >> u16 revision_id;
> >> u16 sub_vendor_id;
> >> u16 sub_device_id;
> >> u16 sub_revision_id;
> >> u8 reserved[6];
> >> u32 serial_number;
> >> u16 fic;
> >> < BLK relevant fields start here <
> >> u16 num_bcw;
> >> u64 bcw_size;
> >> u64 cmd_offset;
> >> u64 cmd_size;
> >> u64 status_offset;
> >> u64 status_size;
> >> u16 flags;
> >> u8 reserved2[6];
> >> };
> >
> > Yes, we do have a DCR entry. But we do not have a SPA-DCR.
>
> Got it. will fix.
Attached is an example implementation of the NFIT table with 2
battery-backed NVDIMM cards, which I have used for testing. I hope this
provides a good example of an NFIT table with SPA(PMEM), MEMDEV and DCR
entries, which allows optional _DSMs for battery-backed NVDIMMs as
necessary.
HP is also defining _DSM method for battery-backed NVDIMMs, and will
share the spec when it is ready.
Thanks,
-Toshi
Thanks,
-Toshi
��* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
* N V D I M M F i r m w a r e I n t e r f a c e
T a b l e
*
* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
N F I T a d d r e s s
. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .
0 x 0 0 0 0 0 0 0 0 7 B 7 E 7 0 0 0
T a b l e H e a d e r :
S i g n a t u r e
. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .
' N F I T '
L e n g t h
. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .
0 x 0 0 0 0 0 1 3 8
R e v i s i o n
. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .
0 x 0 1
C h e c k s u m
. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .
0 x 4 A
O E M I D
. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .
' H P '
O E M T a b l e I D
. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .
' P r o L i a n t '
O E M R e v i s i o n
. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .
0 x 0 0 0 0 0 0 0 1
C r e a t o r I D
. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .
' H P '
C r e a t o r R e v i s i o n
. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .
0 x 0 0 0 0 0 0 0 1
T a b l e C o n t e n t s :
* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
* N F I T S y s t e m P h y s i c a l
A d d r e s s R a n g e S t r u c t u r e
*
* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
R a n g e I n d e x
. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .
0 x 0 0 0 1
F l a g s
Re: [Linux-nvdimm] [PATCH 04/21] nd: create an 'nd_bus' from an 'nfit_desc'
On Wed, 2015-04-22 at 13:00 -0700, Dan Williams wrote:
On Wed, Apr 22, 2015 at 12:38 PM, Toshi Kani toshi.k...@hp.com wrote:
On Wed, 2015-04-22 at 12:28 -0700, Dan Williams wrote:
On Wed, Apr 22, 2015 at 11:23 AM, Toshi Kani toshi.k...@hp.com wrote:
On Wed, 2015-04-22 at 11:20 -0700, Dan Williams wrote:
On Wed, Apr 22, 2015 at 11:00 AM, Linda Knippers
linda.knipp...@hp.com wrote:
Wait, point of clarification, DCRs (dimm-control-regions) have RFICs,
not MEMDEVs (memory-device-to-spa-mapping). Toshi's original report
was that an NFIT with a SPA+MEMDEV was failing to enable a PMEM
device. That specific problem can be fixed by either deleting the
MEMDEV, or adding a DCR.
By a DCR, do you mean a DCR structure or SPA with Control Region GUID?
Hmm, I meant a DCR as defined below. I agree you would not need a
SPA-DCR.
Adding a DCR structure does not solve this issue since it requires SPA
with Control Region GUID, which battery-backed DIMMs do not have.
I would not go that far, half of a DCR entry is relevant for any
NVDIMM, and half is only relevant if a DIMM offers BLK access:
struct acpi_nfit_dcr {
u16 type;
u16 length;
u16 dcr_index;
u16 vendor_id;
u16 device_id;
u16 revision_id;
u16 sub_vendor_id;
u16 sub_device_id;
u16 sub_revision_id;
u8 reserved[6];
u32 serial_number;
u16 fic;
BLK relevant fields start here
u16 num_bcw;
u64 bcw_size;
u64 cmd_offset;
u64 cmd_size;
u64 status_offset;
u64 status_size;
u16 flags;
u8 reserved2[6];
};
Yes, we do have a DCR entry. But we do not have a SPA-DCR.
Got it. will fix.
Attached is an example implementation of the NFIT table with 2
battery-backed NVDIMM cards, which I have used for testing. I hope this
provides a good example of an NFIT table with SPA(PMEM), MEMDEV and DCR
entries, which allows optional _DSMs for battery-backed NVDIMMs as
necessary.
HP is also defining _DSM method for battery-backed NVDIMMs, and will
share the spec when it is ready.
Thanks,
-Toshi
Thanks,
-Toshi
��* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
* N V D I M M F i r m w a r e I n t e r f a c e
T a b l e
*
* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
N F I T a d d r e s s
. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .
0 x 0 0 0 0 0 0 0 0 7 B 7 E 7 0 0 0
T a b l e H e a d e r :
S i g n a t u r e
. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .
' N F I T '
L e n g t h
. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .
0 x 0 0 0 0 0 1 3 8
R e v i s i o n
. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .
0 x 0 1
C h e c k s u m
. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .
0 x 4 A
O E M I D
. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .
' H P '
O E M T a b l e I D
. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .
' P r o L i a n t '
O E M R e v i s i o n
. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .
0 x 0 0 0 0 0 0 0 1
C r e a t o r I D
. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .
' H P '
C r e a t o r R e v i s i o n
. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .
0 x 0 0 0 0 0 0 0 1
T a b l e C o n t e n t s :
* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
* N F I T S y s t e m P h y s i c a l
A d d r e s s R a n g e S t r u c t u r e
*
* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
R a n g e I n d e x
. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .
0 x 0 0 0 1
F l a g s
. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .
0 x 0 0 0 2
H o t p l u g
Re: [Linux-nvdimm] [PATCH 04/21] nd: create an 'nd_bus' from an 'nfit_desc'
On Tue, 2015-04-28 at 10:47 -0600, Toshi Kani wrote:
On Wed, 2015-04-22 at 13:00 -0700, Dan Williams wrote:
On Wed, Apr 22, 2015 at 12:38 PM, Toshi Kani toshi.k...@hp.com wrote:
On Wed, 2015-04-22 at 12:28 -0700, Dan Williams wrote:
On Wed, Apr 22, 2015 at 11:23 AM, Toshi Kani toshi.k...@hp.com wrote:
On Wed, 2015-04-22 at 11:20 -0700, Dan Williams wrote:
On Wed, Apr 22, 2015 at 11:00 AM, Linda Knippers
linda.knipp...@hp.com wrote:
Wait, point of clarification, DCRs (dimm-control-regions) have RFICs,
not MEMDEVs (memory-device-to-spa-mapping). Toshi's original report
was that an NFIT with a SPA+MEMDEV was failing to enable a PMEM
device. That specific problem can be fixed by either deleting the
MEMDEV, or adding a DCR.
By a DCR, do you mean a DCR structure or SPA with Control Region GUID?
Hmm, I meant a DCR as defined below. I agree you would not need a
SPA-DCR.
Adding a DCR structure does not solve this issue since it requires SPA
with Control Region GUID, which battery-backed DIMMs do not have.
I would not go that far, half of a DCR entry is relevant for any
NVDIMM, and half is only relevant if a DIMM offers BLK access:
struct acpi_nfit_dcr {
u16 type;
u16 length;
u16 dcr_index;
u16 vendor_id;
u16 device_id;
u16 revision_id;
u16 sub_vendor_id;
u16 sub_device_id;
u16 sub_revision_id;
u8 reserved[6];
u32 serial_number;
u16 fic;
BLK relevant fields start here
u16 num_bcw;
u64 bcw_size;
u64 cmd_offset;
u64 cmd_size;
u64 status_offset;
u64 status_size;
u16 flags;
u8 reserved2[6];
};
Yes, we do have a DCR entry. But we do not have a SPA-DCR.
Got it. will fix.
Attached is an example implementation of the NFIT table with 2
battery-backed NVDIMM cards, which I have used for testing. I hope this
provides a good example of an NFIT table with SPA(PMEM), MEMDEV and DCR
entries, which allows optional _DSMs for battery-backed NVDIMMs as
necessary.
HP is also defining _DSM method for battery-backed NVDIMMs, and will
share the spec when it is ready.
Sorry, using .txt extension to a Linux text file caused my mailer to
perform some unnecessary conversion... Attached is the same file without
.txt this time.
-Toshi
hp_nfit
Description: Binary data
Re: [Linux-nvdimm] [PATCH 04/21] nd: create an 'nd_bus' from an 'nfit_desc'
On Wed, Apr 22, 2015 at 12:38 PM, Toshi Kani wrote:
> On Wed, 2015-04-22 at 12:28 -0700, Dan Williams wrote:
>> On Wed, Apr 22, 2015 at 11:23 AM, Toshi Kani wrote:
>> > On Wed, 2015-04-22 at 11:20 -0700, Dan Williams wrote:
>> >> On Wed, Apr 22, 2015 at 11:00 AM, Linda Knippers
>> >> wrote:
>> >> Wait, point of clarification, DCRs (dimm-control-regions) have RFICs,
>> >> not MEMDEVs (memory-device-to-spa-mapping). Toshi's original report
>> >> was that an NFIT with a SPA+MEMDEV was failing to enable a PMEM
>> >> device. That specific problem can be fixed by either deleting the
>> >> MEMDEV, or adding a DCR.
>> >
>> > By a DCR, do you mean a DCR structure or SPA with Control Region GUID?
>>
>> Hmm, I meant a DCR as defined below. I agree you would not need a "SPA-DCR".
>>
>> > Adding a DCR structure does not solve this issue since it requires SPA
>> > with Control Region GUID, which battery-backed DIMMs do not have.
>>
>> I would not go that far, half of a DCR entry is relevant for any
>> NVDIMM, and half is only relevant if a DIMM offers BLK access:
>>
>> struct acpi_nfit_dcr {
>> u16 type;
>> u16 length;
>> u16 dcr_index;
>> u16 vendor_id;
>> u16 device_id;
>> u16 revision_id;
>> u16 sub_vendor_id;
>> u16 sub_device_id;
>> u16 sub_revision_id;
>> u8 reserved[6];
>> u32 serial_number;
>> u16 fic;
>> < BLK relevant fields start here <
>> u16 num_bcw;
>> u64 bcw_size;
>> u64 cmd_offset;
>> u64 cmd_size;
>> u64 status_offset;
>> u64 status_size;
>> u16 flags;
>> u8 reserved2[6];
>> };
>
> Yes, we do have a DCR entry. But we do not have a SPA-DCR.
Got it. will fix.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Linux-nvdimm] [PATCH 04/21] nd: create an 'nd_bus' from an 'nfit_desc'
On Wed, 2015-04-22 at 12:28 -0700, Dan Williams wrote:
> On Wed, Apr 22, 2015 at 11:23 AM, Toshi Kani wrote:
> > On Wed, 2015-04-22 at 11:20 -0700, Dan Williams wrote:
> >> On Wed, Apr 22, 2015 at 11:00 AM, Linda Knippers
> >> wrote:
> >> Wait, point of clarification, DCRs (dimm-control-regions) have RFICs,
> >> not MEMDEVs (memory-device-to-spa-mapping). Toshi's original report
> >> was that an NFIT with a SPA+MEMDEV was failing to enable a PMEM
> >> device. That specific problem can be fixed by either deleting the
> >> MEMDEV, or adding a DCR.
> >
> > By a DCR, do you mean a DCR structure or SPA with Control Region GUID?
>
> Hmm, I meant a DCR as defined below. I agree you would not need a "SPA-DCR".
>
> > Adding a DCR structure does not solve this issue since it requires SPA
> > with Control Region GUID, which battery-backed DIMMs do not have.
>
> I would not go that far, half of a DCR entry is relevant for any
> NVDIMM, and half is only relevant if a DIMM offers BLK access:
>
> struct acpi_nfit_dcr {
> u16 type;
> u16 length;
> u16 dcr_index;
> u16 vendor_id;
> u16 device_id;
> u16 revision_id;
> u16 sub_vendor_id;
> u16 sub_device_id;
> u16 sub_revision_id;
> u8 reserved[6];
> u32 serial_number;
> u16 fic;
> < BLK relevant fields start here <
> u16 num_bcw;
> u64 bcw_size;
> u64 cmd_offset;
> u64 cmd_size;
> u64 status_offset;
> u64 status_size;
> u16 flags;
> u8 reserved2[6];
> };
Yes, we do have a DCR entry. But we do not have a SPA-DCR.
The previous issue I reported to nd_mem_init() was caused by the fact
that there was no "SPA-DCR". nd_mem_init() requires SPA-DCR to
initialize nd_mem objects.
> >> Of course, if you add a DCR with a different intended DSM layout than
> >> the DSM-example-interface the driver will need to add support for
> >> handling that case.
> >
> > Yes, we consider to add different _DSMs for management. We do not need
> > the nd_acpi driver to support it now, but we need this framework to work
> > without the DSM-example-interface present.
> >
>
> One possible workaround is that I could ignore MEMDEV entries that do
> not have a corresponding DCR. This would enable nd_namespace_io
> devices to be surfaced for your use case. Would that work for you?
> I.e. do you need the nfit_handle exposed?
We have MEMDEV entries and their corresponding DCR entries. ACPI 6.0
states that NVDIMM control region structure index must contain a
non-zero value in a MEMDEV entry, so I think they must correspond.
Yes, we need this framework to enumerate all entries.
Thanks,
-Toshi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Linux-nvdimm] [PATCH 04/21] nd: create an 'nd_bus' from an 'nfit_desc'
On Wed, Apr 22, 2015 at 11:23 AM, Toshi Kani wrote:
> On Wed, 2015-04-22 at 11:20 -0700, Dan Williams wrote:
>> On Wed, Apr 22, 2015 at 11:00 AM, Linda Knippers
>> wrote:
>> Wait, point of clarification, DCRs (dimm-control-regions) have RFICs,
>> not MEMDEVs (memory-device-to-spa-mapping). Toshi's original report
>> was that an NFIT with a SPA+MEMDEV was failing to enable a PMEM
>> device. That specific problem can be fixed by either deleting the
>> MEMDEV, or adding a DCR.
>
> By a DCR, do you mean a DCR structure or SPA with Control Region GUID?
Hmm, I meant a DCR as defined below. I agree you would not need a "SPA-DCR".
> Adding a DCR structure does not solve this issue since it requires SPA
> with Control Region GUID, which battery-backed DIMMs do not have.
I would not go that far, half of a DCR entry is relevant for any
NVDIMM, and half is only relevant if a DIMM offers BLK access:
struct acpi_nfit_dcr {
u16 type;
u16 length;
u16 dcr_index;
u16 vendor_id;
u16 device_id;
u16 revision_id;
u16 sub_vendor_id;
u16 sub_device_id;
u16 sub_revision_id;
u8 reserved[6];
u32 serial_number;
u16 fic;
< BLK relevant fields start here <
u16 num_bcw;
u64 bcw_size;
u64 cmd_offset;
u64 cmd_size;
u64 status_offset;
u64 status_size;
u16 flags;
u8 reserved2[6];
};
>> Of course, if you add a DCR with a different intended DSM layout than
>> the DSM-example-interface the driver will need to add support for
>> handling that case.
>
> Yes, we consider to add different _DSMs for management. We do not need
> the nd_acpi driver to support it now, but we need this framework to work
> without the DSM-example-interface present.
>
One possible workaround is that I could ignore MEMDEV entries that do
not have a corresponding DCR. This would enable nd_namespace_io
devices to be surfaced for your use case. Would that work for you?
I.e. do you need the nfit_handle exposed?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Linux-nvdimm] [PATCH 04/21] nd: create an 'nd_bus' from an 'nfit_desc'
On Wed, 2015-04-22 at 11:20 -0700, Dan Williams wrote:
> On Wed, Apr 22, 2015 at 11:00 AM, Linda Knippers
> wrote:
> > On 4/22/2015 1:03 PM, Dan Williams wrote:
> >> On Wed, Apr 22, 2015 at 9:39 AM, Toshi Kani wrote:
> >>> On Tue, 2015-04-21 at 13:35 -0700, Dan Williams wrote:
> On Tue, Apr 21, 2015 at 12:55 PM, Toshi Kani wrote:
> > On Tue, 2015-04-21 at 12:58 -0700, Dan Williams wrote:
> >> On Tue, Apr 21, 2015 at 12:35 PM, Toshi Kani wrote:
> >>> On Fri, 2015-04-17 at 21:35 -0400, Dan Williams wrote:
> >>> :
> +
> +static int nd_mem_init(struct nd_bus *nd_bus)
> +{
> + struct nd_spa *nd_spa;
> +
> + /*
> + * For each SPA-DCR address range find its corresponding
> + * MEMDEV(s). From each MEMDEV find the corresponding DCR.
> + * Then, try to find a SPA-BDW and a corresponding BDW that
> + * references the DCR. Throw it all into an nd_mem object.
> + * Note, that BDWs are optional.
> + */
> + list_for_each_entry(nd_spa, _bus->spas, list) {
> + u16 spa_index = readw(_spa->nfit_spa->spa_index);
> + int type = nfit_spa_type(nd_spa->nfit_spa);
> + struct nd_mem *nd_mem, *found;
> + struct nd_memdev *nd_memdev;
> + u16 dcr_index;
> +
> + if (type != NFIT_SPA_DCR)
> + continue;
> >>>
> >>> This function requires NFIT_SPA_DCR, SPA Range Structure with NVDIMM
> >>> Control Region GUID, for initializing an nd_mem object. However,
> >>> battery-backed DIMMs do not have such control region SPA. IIUC, the
> >>> NFIT spec does not require NFIT_SPA_DCR.
> >>>
> >>> Can you change this function to work with NFIT_SPA_PM as well?
> >>
> >> NFIT_SPA_PM ranges are handled separately from nd_mem_init(). See
> >> nd_region_create() in patch 10.
> >
> > If nd_mem_init() does not initialize nd_mem objects, nd_bus_probe() in
> > core.c fails in nd_bus_init_interleave_sets() and skips all subsequent
> > nd_bus_xxx() calls. So, nd_region_create() won't be called.
> >
> > nd_bus_init_interleave_sets() fails because init_interleave_set()
> > returns -ENODEV if (!nd_mem).
>
> Ah, ok your test case is specifying PMEM backed by memory device
> info. We have a test case for simple ranges (nfit_test1_setup()), but
> it doesn't hit this bug because it does not specify any memory-device
> tables.
> >>>
> >>> Yes, we have NFIT table with SPA range (PM), memory device to SPA, and
> >>> NVDIMM control region structures. With the memory device to SPA
> >>> structure, this code requires full sets of information, including the
> >>> namespace label data in _DSM [1], which is outside of ACPI 6.0 and is
> >>> optional. Battery-backed DIMMs do not have such label data.
> >>
> >> This is what "nd_namespace_io" devices are for, they do not require labels.
> >>
> >> Question, if you don't have labels and you don't have DSMs then why
> >> publish a MEMDEV table at all? Why not simply publish an anonymous
> >> range? See nfit_test1_setup().
> >
> > The MEMDEV table provides useful information, and there may be _DSMs,
> > perhaps just not the same _DSM as some other devices.
> >
> >>> It needs
> >>> to work with NFIT table with these structures without this _DSM or with
> >>> a different type of _DSM which this code may or may not need to support.
> >>> It should also check Region Format Interface Code (RFIC) in the NVDIMM
> >>> control region structure before assuming this _DSM is present to
> >>> implement RFIC 0x0201.
> >>
> >> Ok I can look into adding this check, but I don't think it is
> >> necessary if you simply refrain from publishing a MEMDEV entry.
> >
> > But we need the MEMDEV. And as Toshi mentions, we could have other
> > RFICs with other _DSMs than your example. That's why there is an RFIC.
>
> Wait, point of clarification, DCRs (dimm-control-regions) have RFICs,
> not MEMDEVs (memory-device-to-spa-mapping). Toshi's original report
> was that an NFIT with a SPA+MEMDEV was failing to enable a PMEM
> device. That specific problem can be fixed by either deleting the
> MEMDEV, or adding a DCR.
By a DCR, do you mean a DCR structure or SPA with Control Region GUID?
Adding a DCR structure does not solve this issue since it requires SPA
with Control Region GUID, which battery-backed DIMMs do not have.
> Of course, if you add a DCR with a different intended DSM layout than
> the DSM-example-interface the driver will need to add support for
> handling that case.
Yes, we consider to add different _DSMs for management. We do not need
the nd_acpi driver to support it now, but we need this framework to work
without the DSM-example-interface present.
Thanks,
-Toshi
--
To unsubscribe from this
Re: [Linux-nvdimm] [PATCH 04/21] nd: create an 'nd_bus' from an 'nfit_desc'
On Wed, Apr 22, 2015 at 11:00 AM, Linda Knippers wrote:
> On 4/22/2015 1:03 PM, Dan Williams wrote:
>> On Wed, Apr 22, 2015 at 9:39 AM, Toshi Kani wrote:
>>> On Tue, 2015-04-21 at 13:35 -0700, Dan Williams wrote:
On Tue, Apr 21, 2015 at 12:55 PM, Toshi Kani wrote:
> On Tue, 2015-04-21 at 12:58 -0700, Dan Williams wrote:
>> On Tue, Apr 21, 2015 at 12:35 PM, Toshi Kani wrote:
>>> On Fri, 2015-04-17 at 21:35 -0400, Dan Williams wrote:
>>> :
+
+static int nd_mem_init(struct nd_bus *nd_bus)
+{
+ struct nd_spa *nd_spa;
+
+ /*
+ * For each SPA-DCR address range find its corresponding
+ * MEMDEV(s). From each MEMDEV find the corresponding DCR.
+ * Then, try to find a SPA-BDW and a corresponding BDW that
+ * references the DCR. Throw it all into an nd_mem object.
+ * Note, that BDWs are optional.
+ */
+ list_for_each_entry(nd_spa, _bus->spas, list) {
+ u16 spa_index = readw(_spa->nfit_spa->spa_index);
+ int type = nfit_spa_type(nd_spa->nfit_spa);
+ struct nd_mem *nd_mem, *found;
+ struct nd_memdev *nd_memdev;
+ u16 dcr_index;
+
+ if (type != NFIT_SPA_DCR)
+ continue;
>>>
>>> This function requires NFIT_SPA_DCR, SPA Range Structure with NVDIMM
>>> Control Region GUID, for initializing an nd_mem object. However,
>>> battery-backed DIMMs do not have such control region SPA. IIUC, the
>>> NFIT spec does not require NFIT_SPA_DCR.
>>>
>>> Can you change this function to work with NFIT_SPA_PM as well?
>>
>> NFIT_SPA_PM ranges are handled separately from nd_mem_init(). See
>> nd_region_create() in patch 10.
>
> If nd_mem_init() does not initialize nd_mem objects, nd_bus_probe() in
> core.c fails in nd_bus_init_interleave_sets() and skips all subsequent
> nd_bus_xxx() calls. So, nd_region_create() won't be called.
>
> nd_bus_init_interleave_sets() fails because init_interleave_set()
> returns -ENODEV if (!nd_mem).
Ah, ok your test case is specifying PMEM backed by memory device
info. We have a test case for simple ranges (nfit_test1_setup()), but
it doesn't hit this bug because it does not specify any memory-device
tables.
>>>
>>> Yes, we have NFIT table with SPA range (PM), memory device to SPA, and
>>> NVDIMM control region structures. With the memory device to SPA
>>> structure, this code requires full sets of information, including the
>>> namespace label data in _DSM [1], which is outside of ACPI 6.0 and is
>>> optional. Battery-backed DIMMs do not have such label data.
>>
>> This is what "nd_namespace_io" devices are for, they do not require labels.
>>
>> Question, if you don't have labels and you don't have DSMs then why
>> publish a MEMDEV table at all? Why not simply publish an anonymous
>> range? See nfit_test1_setup().
>
> The MEMDEV table provides useful information, and there may be _DSMs,
> perhaps just not the same _DSM as some other devices.
>
>>> It needs
>>> to work with NFIT table with these structures without this _DSM or with
>>> a different type of _DSM which this code may or may not need to support.
>>> It should also check Region Format Interface Code (RFIC) in the NVDIMM
>>> control region structure before assuming this _DSM is present to
>>> implement RFIC 0x0201.
>>
>> Ok I can look into adding this check, but I don't think it is
>> necessary if you simply refrain from publishing a MEMDEV entry.
>
> But we need the MEMDEV. And as Toshi mentions, we could have other
> RFICs with other _DSMs than your example. That's why there is an RFIC.
Wait, point of clarification, DCRs (dimm-control-regions) have RFICs,
not MEMDEVs (memory-device-to-spa-mapping). Toshi's original report
was that an NFIT with a SPA+MEMDEV was failing to enable a PMEM
device. That specific problem can be fixed by either deleting the
MEMDEV, or adding a DCR.
Of course, if you add a DCR with a different intended DSM layout than
the DSM-example-interface the driver will need to add support for
handling that case.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Linux-nvdimm] [PATCH 04/21] nd: create an 'nd_bus' from an 'nfit_desc'
On 4/22/2015 1:03 PM, Dan Williams wrote:
> On Wed, Apr 22, 2015 at 9:39 AM, Toshi Kani wrote:
>> On Tue, 2015-04-21 at 13:35 -0700, Dan Williams wrote:
>>> On Tue, Apr 21, 2015 at 12:55 PM, Toshi Kani wrote:
On Tue, 2015-04-21 at 12:58 -0700, Dan Williams wrote:
> On Tue, Apr 21, 2015 at 12:35 PM, Toshi Kani wrote:
>> On Fri, 2015-04-17 at 21:35 -0400, Dan Williams wrote:
>> :
>>> +
>>> +static int nd_mem_init(struct nd_bus *nd_bus)
>>> +{
>>> + struct nd_spa *nd_spa;
>>> +
>>> + /*
>>> + * For each SPA-DCR address range find its corresponding
>>> + * MEMDEV(s). From each MEMDEV find the corresponding DCR.
>>> + * Then, try to find a SPA-BDW and a corresponding BDW that
>>> + * references the DCR. Throw it all into an nd_mem object.
>>> + * Note, that BDWs are optional.
>>> + */
>>> + list_for_each_entry(nd_spa, _bus->spas, list) {
>>> + u16 spa_index = readw(_spa->nfit_spa->spa_index);
>>> + int type = nfit_spa_type(nd_spa->nfit_spa);
>>> + struct nd_mem *nd_mem, *found;
>>> + struct nd_memdev *nd_memdev;
>>> + u16 dcr_index;
>>> +
>>> + if (type != NFIT_SPA_DCR)
>>> + continue;
>>
>> This function requires NFIT_SPA_DCR, SPA Range Structure with NVDIMM
>> Control Region GUID, for initializing an nd_mem object. However,
>> battery-backed DIMMs do not have such control region SPA. IIUC, the
>> NFIT spec does not require NFIT_SPA_DCR.
>>
>> Can you change this function to work with NFIT_SPA_PM as well?
>
> NFIT_SPA_PM ranges are handled separately from nd_mem_init(). See
> nd_region_create() in patch 10.
If nd_mem_init() does not initialize nd_mem objects, nd_bus_probe() in
core.c fails in nd_bus_init_interleave_sets() and skips all subsequent
nd_bus_xxx() calls. So, nd_region_create() won't be called.
nd_bus_init_interleave_sets() fails because init_interleave_set()
returns -ENODEV if (!nd_mem).
>>>
>>> Ah, ok your test case is specifying PMEM backed by memory device
>>> info. We have a test case for simple ranges (nfit_test1_setup()), but
>>> it doesn't hit this bug because it does not specify any memory-device
>>> tables.
>>
>> Yes, we have NFIT table with SPA range (PM), memory device to SPA, and
>> NVDIMM control region structures. With the memory device to SPA
>> structure, this code requires full sets of information, including the
>> namespace label data in _DSM [1], which is outside of ACPI 6.0 and is
>> optional. Battery-backed DIMMs do not have such label data.
>
> This is what "nd_namespace_io" devices are for, they do not require labels.
>
> Question, if you don't have labels and you don't have DSMs then why
> publish a MEMDEV table at all? Why not simply publish an anonymous
> range? See nfit_test1_setup().
The MEMDEV table provides useful information, and there may be _DSMs,
perhaps just not the same _DSM as some other devices.
>> It needs
>> to work with NFIT table with these structures without this _DSM or with
>> a different type of _DSM which this code may or may not need to support.
>> It should also check Region Format Interface Code (RFIC) in the NVDIMM
>> control region structure before assuming this _DSM is present to
>> implement RFIC 0x0201.
>
> Ok I can look into adding this check, but I don't think it is
> necessary if you simply refrain from publishing a MEMDEV entry.
But we need the MEMDEV. And as Toshi mentions, we could have other
RFICs with other _DSMs than your example. That's why there is an RFIC.
-- ljk
> ___
> Linux-nvdimm mailing list
> linux-nvd...@lists.01.org
> https://lists.01.org/mailman/listinfo/linux-nvdimm
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Linux-nvdimm] [PATCH 04/21] nd: create an 'nd_bus' from an 'nfit_desc'
On Wed, Apr 22, 2015 at 9:39 AM, Toshi Kani wrote:
> On Tue, 2015-04-21 at 13:35 -0700, Dan Williams wrote:
>> On Tue, Apr 21, 2015 at 12:55 PM, Toshi Kani wrote:
>> > On Tue, 2015-04-21 at 12:58 -0700, Dan Williams wrote:
>> >> On Tue, Apr 21, 2015 at 12:35 PM, Toshi Kani wrote:
>> >> > On Fri, 2015-04-17 at 21:35 -0400, Dan Williams wrote:
>> >> > :
>> >> >> +
>> >> >> +static int nd_mem_init(struct nd_bus *nd_bus)
>> >> >> +{
>> >> >> + struct nd_spa *nd_spa;
>> >> >> +
>> >> >> + /*
>> >> >> + * For each SPA-DCR address range find its corresponding
>> >> >> + * MEMDEV(s). From each MEMDEV find the corresponding DCR.
>> >> >> + * Then, try to find a SPA-BDW and a corresponding BDW that
>> >> >> + * references the DCR. Throw it all into an nd_mem object.
>> >> >> + * Note, that BDWs are optional.
>> >> >> + */
>> >> >> + list_for_each_entry(nd_spa, _bus->spas, list) {
>> >> >> + u16 spa_index = readw(_spa->nfit_spa->spa_index);
>> >> >> + int type = nfit_spa_type(nd_spa->nfit_spa);
>> >> >> + struct nd_mem *nd_mem, *found;
>> >> >> + struct nd_memdev *nd_memdev;
>> >> >> + u16 dcr_index;
>> >> >> +
>> >> >> + if (type != NFIT_SPA_DCR)
>> >> >> + continue;
>> >> >
>> >> > This function requires NFIT_SPA_DCR, SPA Range Structure with NVDIMM
>> >> > Control Region GUID, for initializing an nd_mem object. However,
>> >> > battery-backed DIMMs do not have such control region SPA. IIUC, the
>> >> > NFIT spec does not require NFIT_SPA_DCR.
>> >> >
>> >> > Can you change this function to work with NFIT_SPA_PM as well?
>> >>
>> >> NFIT_SPA_PM ranges are handled separately from nd_mem_init(). See
>> >> nd_region_create() in patch 10.
>> >
>> > If nd_mem_init() does not initialize nd_mem objects, nd_bus_probe() in
>> > core.c fails in nd_bus_init_interleave_sets() and skips all subsequent
>> > nd_bus_xxx() calls. So, nd_region_create() won't be called.
>> >
>> > nd_bus_init_interleave_sets() fails because init_interleave_set()
>> > returns -ENODEV if (!nd_mem).
>>
>> Ah, ok your test case is specifying PMEM backed by memory device
>> info. We have a test case for simple ranges (nfit_test1_setup()), but
>> it doesn't hit this bug because it does not specify any memory-device
>> tables.
>
> Yes, we have NFIT table with SPA range (PM), memory device to SPA, and
> NVDIMM control region structures. With the memory device to SPA
> structure, this code requires full sets of information, including the
> namespace label data in _DSM [1], which is outside of ACPI 6.0 and is
> optional. Battery-backed DIMMs do not have such label data.
This is what "nd_namespace_io" devices are for, they do not require labels.
Question, if you don't have labels and you don't have DSMs then why
publish a MEMDEV table at all? Why not simply publish an anonymous
range? See nfit_test1_setup().
> It needs
> to work with NFIT table with these structures without this _DSM or with
> a different type of _DSM which this code may or may not need to support.
> It should also check Region Format Interface Code (RFIC) in the NVDIMM
> control region structure before assuming this _DSM is present to
> implement RFIC 0x0201.
Ok I can look into adding this check, but I don't think it is
necessary if you simply refrain from publishing a MEMDEV entry.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Linux-nvdimm] [PATCH 04/21] nd: create an 'nd_bus' from an 'nfit_desc'
On Tue, 2015-04-21 at 13:35 -0700, Dan Williams wrote:
> On Tue, Apr 21, 2015 at 12:55 PM, Toshi Kani wrote:
> > On Tue, 2015-04-21 at 12:58 -0700, Dan Williams wrote:
> >> On Tue, Apr 21, 2015 at 12:35 PM, Toshi Kani wrote:
> >> > On Fri, 2015-04-17 at 21:35 -0400, Dan Williams wrote:
> >> > :
> >> >> +
> >> >> +static int nd_mem_init(struct nd_bus *nd_bus)
> >> >> +{
> >> >> + struct nd_spa *nd_spa;
> >> >> +
> >> >> + /*
> >> >> + * For each SPA-DCR address range find its corresponding
> >> >> + * MEMDEV(s). From each MEMDEV find the corresponding DCR.
> >> >> + * Then, try to find a SPA-BDW and a corresponding BDW that
> >> >> + * references the DCR. Throw it all into an nd_mem object.
> >> >> + * Note, that BDWs are optional.
> >> >> + */
> >> >> + list_for_each_entry(nd_spa, _bus->spas, list) {
> >> >> + u16 spa_index = readw(_spa->nfit_spa->spa_index);
> >> >> + int type = nfit_spa_type(nd_spa->nfit_spa);
> >> >> + struct nd_mem *nd_mem, *found;
> >> >> + struct nd_memdev *nd_memdev;
> >> >> + u16 dcr_index;
> >> >> +
> >> >> + if (type != NFIT_SPA_DCR)
> >> >> + continue;
> >> >
> >> > This function requires NFIT_SPA_DCR, SPA Range Structure with NVDIMM
> >> > Control Region GUID, for initializing an nd_mem object. However,
> >> > battery-backed DIMMs do not have such control region SPA. IIUC, the
> >> > NFIT spec does not require NFIT_SPA_DCR.
> >> >
> >> > Can you change this function to work with NFIT_SPA_PM as well?
> >>
> >> NFIT_SPA_PM ranges are handled separately from nd_mem_init(). See
> >> nd_region_create() in patch 10.
> >
> > If nd_mem_init() does not initialize nd_mem objects, nd_bus_probe() in
> > core.c fails in nd_bus_init_interleave_sets() and skips all subsequent
> > nd_bus_xxx() calls. So, nd_region_create() won't be called.
> >
> > nd_bus_init_interleave_sets() fails because init_interleave_set()
> > returns -ENODEV if (!nd_mem).
>
> Ah, ok your test case is specifying PMEM backed by memory device
> info. We have a test case for simple ranges (nfit_test1_setup()), but
> it doesn't hit this bug because it does not specify any memory-device
> tables.
Yes, we have NFIT table with SPA range (PM), memory device to SPA, and
NVDIMM control region structures. With the memory device to SPA
structure, this code requires full sets of information, including the
namespace label data in _DSM [1], which is outside of ACPI 6.0 and is
optional. Battery-backed DIMMs do not have such label data. It needs
to work with NFIT table with these structures without this _DSM or with
a different type of _DSM which this code may or may not need to support.
It should also check Region Format Interface Code (RFIC) in the NVDIMM
control region structure before assuming this _DSM is present to
implement RFIC 0x0201.
[1] http://pmem.io/documents/NVDIMM_DSM_Interface_Example.pdf
Thanks,
-Toshi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Linux-nvdimm] [PATCH 04/21] nd: create an 'nd_bus' from an 'nfit_desc'
On Tue, 2015-04-21 at 13:35 -0700, Dan Williams wrote:
On Tue, Apr 21, 2015 at 12:55 PM, Toshi Kani toshi.k...@hp.com wrote:
On Tue, 2015-04-21 at 12:58 -0700, Dan Williams wrote:
On Tue, Apr 21, 2015 at 12:35 PM, Toshi Kani toshi.k...@hp.com wrote:
On Fri, 2015-04-17 at 21:35 -0400, Dan Williams wrote:
:
+
+static int nd_mem_init(struct nd_bus *nd_bus)
+{
+ struct nd_spa *nd_spa;
+
+ /*
+ * For each SPA-DCR address range find its corresponding
+ * MEMDEV(s). From each MEMDEV find the corresponding DCR.
+ * Then, try to find a SPA-BDW and a corresponding BDW that
+ * references the DCR. Throw it all into an nd_mem object.
+ * Note, that BDWs are optional.
+ */
+ list_for_each_entry(nd_spa, nd_bus-spas, list) {
+ u16 spa_index = readw(nd_spa-nfit_spa-spa_index);
+ int type = nfit_spa_type(nd_spa-nfit_spa);
+ struct nd_mem *nd_mem, *found;
+ struct nd_memdev *nd_memdev;
+ u16 dcr_index;
+
+ if (type != NFIT_SPA_DCR)
+ continue;
This function requires NFIT_SPA_DCR, SPA Range Structure with NVDIMM
Control Region GUID, for initializing an nd_mem object. However,
battery-backed DIMMs do not have such control region SPA. IIUC, the
NFIT spec does not require NFIT_SPA_DCR.
Can you change this function to work with NFIT_SPA_PM as well?
NFIT_SPA_PM ranges are handled separately from nd_mem_init(). See
nd_region_create() in patch 10.
If nd_mem_init() does not initialize nd_mem objects, nd_bus_probe() in
core.c fails in nd_bus_init_interleave_sets() and skips all subsequent
nd_bus_xxx() calls. So, nd_region_create() won't be called.
nd_bus_init_interleave_sets() fails because init_interleave_set()
returns -ENODEV if (!nd_mem).
Ah, ok your test case is specifying PMEM backed by memory device
info. We have a test case for simple ranges (nfit_test1_setup()), but
it doesn't hit this bug because it does not specify any memory-device
tables.
Yes, we have NFIT table with SPA range (PM), memory device to SPA, and
NVDIMM control region structures. With the memory device to SPA
structure, this code requires full sets of information, including the
namespace label data in _DSM [1], which is outside of ACPI 6.0 and is
optional. Battery-backed DIMMs do not have such label data. It needs
to work with NFIT table with these structures without this _DSM or with
a different type of _DSM which this code may or may not need to support.
It should also check Region Format Interface Code (RFIC) in the NVDIMM
control region structure before assuming this _DSM is present to
implement RFIC 0x0201.
[1] http://pmem.io/documents/NVDIMM_DSM_Interface_Example.pdf
Thanks,
-Toshi
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Linux-nvdimm] [PATCH 04/21] nd: create an 'nd_bus' from an 'nfit_desc'
On Wed, Apr 22, 2015 at 9:39 AM, Toshi Kani toshi.k...@hp.com wrote:
On Tue, 2015-04-21 at 13:35 -0700, Dan Williams wrote:
On Tue, Apr 21, 2015 at 12:55 PM, Toshi Kani toshi.k...@hp.com wrote:
On Tue, 2015-04-21 at 12:58 -0700, Dan Williams wrote:
On Tue, Apr 21, 2015 at 12:35 PM, Toshi Kani toshi.k...@hp.com wrote:
On Fri, 2015-04-17 at 21:35 -0400, Dan Williams wrote:
:
+
+static int nd_mem_init(struct nd_bus *nd_bus)
+{
+ struct nd_spa *nd_spa;
+
+ /*
+ * For each SPA-DCR address range find its corresponding
+ * MEMDEV(s). From each MEMDEV find the corresponding DCR.
+ * Then, try to find a SPA-BDW and a corresponding BDW that
+ * references the DCR. Throw it all into an nd_mem object.
+ * Note, that BDWs are optional.
+ */
+ list_for_each_entry(nd_spa, nd_bus-spas, list) {
+ u16 spa_index = readw(nd_spa-nfit_spa-spa_index);
+ int type = nfit_spa_type(nd_spa-nfit_spa);
+ struct nd_mem *nd_mem, *found;
+ struct nd_memdev *nd_memdev;
+ u16 dcr_index;
+
+ if (type != NFIT_SPA_DCR)
+ continue;
This function requires NFIT_SPA_DCR, SPA Range Structure with NVDIMM
Control Region GUID, for initializing an nd_mem object. However,
battery-backed DIMMs do not have such control region SPA. IIUC, the
NFIT spec does not require NFIT_SPA_DCR.
Can you change this function to work with NFIT_SPA_PM as well?
NFIT_SPA_PM ranges are handled separately from nd_mem_init(). See
nd_region_create() in patch 10.
If nd_mem_init() does not initialize nd_mem objects, nd_bus_probe() in
core.c fails in nd_bus_init_interleave_sets() and skips all subsequent
nd_bus_xxx() calls. So, nd_region_create() won't be called.
nd_bus_init_interleave_sets() fails because init_interleave_set()
returns -ENODEV if (!nd_mem).
Ah, ok your test case is specifying PMEM backed by memory device
info. We have a test case for simple ranges (nfit_test1_setup()), but
it doesn't hit this bug because it does not specify any memory-device
tables.
Yes, we have NFIT table with SPA range (PM), memory device to SPA, and
NVDIMM control region structures. With the memory device to SPA
structure, this code requires full sets of information, including the
namespace label data in _DSM [1], which is outside of ACPI 6.0 and is
optional. Battery-backed DIMMs do not have such label data.
This is what nd_namespace_io devices are for, they do not require labels.
Question, if you don't have labels and you don't have DSMs then why
publish a MEMDEV table at all? Why not simply publish an anonymous
range? See nfit_test1_setup().
It needs
to work with NFIT table with these structures without this _DSM or with
a different type of _DSM which this code may or may not need to support.
It should also check Region Format Interface Code (RFIC) in the NVDIMM
control region structure before assuming this _DSM is present to
implement RFIC 0x0201.
Ok I can look into adding this check, but I don't think it is
necessary if you simply refrain from publishing a MEMDEV entry.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Linux-nvdimm] [PATCH 04/21] nd: create an 'nd_bus' from an 'nfit_desc'
On 4/22/2015 1:03 PM, Dan Williams wrote:
On Wed, Apr 22, 2015 at 9:39 AM, Toshi Kani toshi.k...@hp.com wrote:
On Tue, 2015-04-21 at 13:35 -0700, Dan Williams wrote:
On Tue, Apr 21, 2015 at 12:55 PM, Toshi Kani toshi.k...@hp.com wrote:
On Tue, 2015-04-21 at 12:58 -0700, Dan Williams wrote:
On Tue, Apr 21, 2015 at 12:35 PM, Toshi Kani toshi.k...@hp.com wrote:
On Fri, 2015-04-17 at 21:35 -0400, Dan Williams wrote:
:
+
+static int nd_mem_init(struct nd_bus *nd_bus)
+{
+ struct nd_spa *nd_spa;
+
+ /*
+ * For each SPA-DCR address range find its corresponding
+ * MEMDEV(s). From each MEMDEV find the corresponding DCR.
+ * Then, try to find a SPA-BDW and a corresponding BDW that
+ * references the DCR. Throw it all into an nd_mem object.
+ * Note, that BDWs are optional.
+ */
+ list_for_each_entry(nd_spa, nd_bus-spas, list) {
+ u16 spa_index = readw(nd_spa-nfit_spa-spa_index);
+ int type = nfit_spa_type(nd_spa-nfit_spa);
+ struct nd_mem *nd_mem, *found;
+ struct nd_memdev *nd_memdev;
+ u16 dcr_index;
+
+ if (type != NFIT_SPA_DCR)
+ continue;
This function requires NFIT_SPA_DCR, SPA Range Structure with NVDIMM
Control Region GUID, for initializing an nd_mem object. However,
battery-backed DIMMs do not have such control region SPA. IIUC, the
NFIT spec does not require NFIT_SPA_DCR.
Can you change this function to work with NFIT_SPA_PM as well?
NFIT_SPA_PM ranges are handled separately from nd_mem_init(). See
nd_region_create() in patch 10.
If nd_mem_init() does not initialize nd_mem objects, nd_bus_probe() in
core.c fails in nd_bus_init_interleave_sets() and skips all subsequent
nd_bus_xxx() calls. So, nd_region_create() won't be called.
nd_bus_init_interleave_sets() fails because init_interleave_set()
returns -ENODEV if (!nd_mem).
Ah, ok your test case is specifying PMEM backed by memory device
info. We have a test case for simple ranges (nfit_test1_setup()), but
it doesn't hit this bug because it does not specify any memory-device
tables.
Yes, we have NFIT table with SPA range (PM), memory device to SPA, and
NVDIMM control region structures. With the memory device to SPA
structure, this code requires full sets of information, including the
namespace label data in _DSM [1], which is outside of ACPI 6.0 and is
optional. Battery-backed DIMMs do not have such label data.
This is what nd_namespace_io devices are for, they do not require labels.
Question, if you don't have labels and you don't have DSMs then why
publish a MEMDEV table at all? Why not simply publish an anonymous
range? See nfit_test1_setup().
The MEMDEV table provides useful information, and there may be _DSMs,
perhaps just not the same _DSM as some other devices.
It needs
to work with NFIT table with these structures without this _DSM or with
a different type of _DSM which this code may or may not need to support.
It should also check Region Format Interface Code (RFIC) in the NVDIMM
control region structure before assuming this _DSM is present to
implement RFIC 0x0201.
Ok I can look into adding this check, but I don't think it is
necessary if you simply refrain from publishing a MEMDEV entry.
But we need the MEMDEV. And as Toshi mentions, we could have other
RFICs with other _DSMs than your example. That's why there is an RFIC.
-- ljk
___
Linux-nvdimm mailing list
linux-nvd...@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Linux-nvdimm] [PATCH 04/21] nd: create an 'nd_bus' from an 'nfit_desc'
On Wed, 2015-04-22 at 11:20 -0700, Dan Williams wrote:
On Wed, Apr 22, 2015 at 11:00 AM, Linda Knippers linda.knipp...@hp.com
wrote:
On 4/22/2015 1:03 PM, Dan Williams wrote:
On Wed, Apr 22, 2015 at 9:39 AM, Toshi Kani toshi.k...@hp.com wrote:
On Tue, 2015-04-21 at 13:35 -0700, Dan Williams wrote:
On Tue, Apr 21, 2015 at 12:55 PM, Toshi Kani toshi.k...@hp.com wrote:
On Tue, 2015-04-21 at 12:58 -0700, Dan Williams wrote:
On Tue, Apr 21, 2015 at 12:35 PM, Toshi Kani toshi.k...@hp.com wrote:
On Fri, 2015-04-17 at 21:35 -0400, Dan Williams wrote:
:
+
+static int nd_mem_init(struct nd_bus *nd_bus)
+{
+ struct nd_spa *nd_spa;
+
+ /*
+ * For each SPA-DCR address range find its corresponding
+ * MEMDEV(s). From each MEMDEV find the corresponding DCR.
+ * Then, try to find a SPA-BDW and a corresponding BDW that
+ * references the DCR. Throw it all into an nd_mem object.
+ * Note, that BDWs are optional.
+ */
+ list_for_each_entry(nd_spa, nd_bus-spas, list) {
+ u16 spa_index = readw(nd_spa-nfit_spa-spa_index);
+ int type = nfit_spa_type(nd_spa-nfit_spa);
+ struct nd_mem *nd_mem, *found;
+ struct nd_memdev *nd_memdev;
+ u16 dcr_index;
+
+ if (type != NFIT_SPA_DCR)
+ continue;
This function requires NFIT_SPA_DCR, SPA Range Structure with NVDIMM
Control Region GUID, for initializing an nd_mem object. However,
battery-backed DIMMs do not have such control region SPA. IIUC, the
NFIT spec does not require NFIT_SPA_DCR.
Can you change this function to work with NFIT_SPA_PM as well?
NFIT_SPA_PM ranges are handled separately from nd_mem_init(). See
nd_region_create() in patch 10.
If nd_mem_init() does not initialize nd_mem objects, nd_bus_probe() in
core.c fails in nd_bus_init_interleave_sets() and skips all subsequent
nd_bus_xxx() calls. So, nd_region_create() won't be called.
nd_bus_init_interleave_sets() fails because init_interleave_set()
returns -ENODEV if (!nd_mem).
Ah, ok your test case is specifying PMEM backed by memory device
info. We have a test case for simple ranges (nfit_test1_setup()), but
it doesn't hit this bug because it does not specify any memory-device
tables.
Yes, we have NFIT table with SPA range (PM), memory device to SPA, and
NVDIMM control region structures. With the memory device to SPA
structure, this code requires full sets of information, including the
namespace label data in _DSM [1], which is outside of ACPI 6.0 and is
optional. Battery-backed DIMMs do not have such label data.
This is what nd_namespace_io devices are for, they do not require labels.
Question, if you don't have labels and you don't have DSMs then why
publish a MEMDEV table at all? Why not simply publish an anonymous
range? See nfit_test1_setup().
The MEMDEV table provides useful information, and there may be _DSMs,
perhaps just not the same _DSM as some other devices.
It needs
to work with NFIT table with these structures without this _DSM or with
a different type of _DSM which this code may or may not need to support.
It should also check Region Format Interface Code (RFIC) in the NVDIMM
control region structure before assuming this _DSM is present to
implement RFIC 0x0201.
Ok I can look into adding this check, but I don't think it is
necessary if you simply refrain from publishing a MEMDEV entry.
But we need the MEMDEV. And as Toshi mentions, we could have other
RFICs with other _DSMs than your example. That's why there is an RFIC.
Wait, point of clarification, DCRs (dimm-control-regions) have RFICs,
not MEMDEVs (memory-device-to-spa-mapping). Toshi's original report
was that an NFIT with a SPA+MEMDEV was failing to enable a PMEM
device. That specific problem can be fixed by either deleting the
MEMDEV, or adding a DCR.
By a DCR, do you mean a DCR structure or SPA with Control Region GUID?
Adding a DCR structure does not solve this issue since it requires SPA
with Control Region GUID, which battery-backed DIMMs do not have.
Of course, if you add a DCR with a different intended DSM layout than
the DSM-example-interface the driver will need to add support for
handling that case.
Yes, we consider to add different _DSMs for management. We do not need
the nd_acpi driver to support it now, but we need this framework to work
without the DSM-example-interface present.
Thanks,
-Toshi
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Linux-nvdimm] [PATCH 04/21] nd: create an 'nd_bus' from an 'nfit_desc'
On Wed, Apr 22, 2015 at 11:00 AM, Linda Knippers linda.knipp...@hp.com wrote:
On 4/22/2015 1:03 PM, Dan Williams wrote:
On Wed, Apr 22, 2015 at 9:39 AM, Toshi Kani toshi.k...@hp.com wrote:
On Tue, 2015-04-21 at 13:35 -0700, Dan Williams wrote:
On Tue, Apr 21, 2015 at 12:55 PM, Toshi Kani toshi.k...@hp.com wrote:
On Tue, 2015-04-21 at 12:58 -0700, Dan Williams wrote:
On Tue, Apr 21, 2015 at 12:35 PM, Toshi Kani toshi.k...@hp.com wrote:
On Fri, 2015-04-17 at 21:35 -0400, Dan Williams wrote:
:
+
+static int nd_mem_init(struct nd_bus *nd_bus)
+{
+ struct nd_spa *nd_spa;
+
+ /*
+ * For each SPA-DCR address range find its corresponding
+ * MEMDEV(s). From each MEMDEV find the corresponding DCR.
+ * Then, try to find a SPA-BDW and a corresponding BDW that
+ * references the DCR. Throw it all into an nd_mem object.
+ * Note, that BDWs are optional.
+ */
+ list_for_each_entry(nd_spa, nd_bus-spas, list) {
+ u16 spa_index = readw(nd_spa-nfit_spa-spa_index);
+ int type = nfit_spa_type(nd_spa-nfit_spa);
+ struct nd_mem *nd_mem, *found;
+ struct nd_memdev *nd_memdev;
+ u16 dcr_index;
+
+ if (type != NFIT_SPA_DCR)
+ continue;
This function requires NFIT_SPA_DCR, SPA Range Structure with NVDIMM
Control Region GUID, for initializing an nd_mem object. However,
battery-backed DIMMs do not have such control region SPA. IIUC, the
NFIT spec does not require NFIT_SPA_DCR.
Can you change this function to work with NFIT_SPA_PM as well?
NFIT_SPA_PM ranges are handled separately from nd_mem_init(). See
nd_region_create() in patch 10.
If nd_mem_init() does not initialize nd_mem objects, nd_bus_probe() in
core.c fails in nd_bus_init_interleave_sets() and skips all subsequent
nd_bus_xxx() calls. So, nd_region_create() won't be called.
nd_bus_init_interleave_sets() fails because init_interleave_set()
returns -ENODEV if (!nd_mem).
Ah, ok your test case is specifying PMEM backed by memory device
info. We have a test case for simple ranges (nfit_test1_setup()), but
it doesn't hit this bug because it does not specify any memory-device
tables.
Yes, we have NFIT table with SPA range (PM), memory device to SPA, and
NVDIMM control region structures. With the memory device to SPA
structure, this code requires full sets of information, including the
namespace label data in _DSM [1], which is outside of ACPI 6.0 and is
optional. Battery-backed DIMMs do not have such label data.
This is what nd_namespace_io devices are for, they do not require labels.
Question, if you don't have labels and you don't have DSMs then why
publish a MEMDEV table at all? Why not simply publish an anonymous
range? See nfit_test1_setup().
The MEMDEV table provides useful information, and there may be _DSMs,
perhaps just not the same _DSM as some other devices.
It needs
to work with NFIT table with these structures without this _DSM or with
a different type of _DSM which this code may or may not need to support.
It should also check Region Format Interface Code (RFIC) in the NVDIMM
control region structure before assuming this _DSM is present to
implement RFIC 0x0201.
Ok I can look into adding this check, but I don't think it is
necessary if you simply refrain from publishing a MEMDEV entry.
But we need the MEMDEV. And as Toshi mentions, we could have other
RFICs with other _DSMs than your example. That's why there is an RFIC.
Wait, point of clarification, DCRs (dimm-control-regions) have RFICs,
not MEMDEVs (memory-device-to-spa-mapping). Toshi's original report
was that an NFIT with a SPA+MEMDEV was failing to enable a PMEM
device. That specific problem can be fixed by either deleting the
MEMDEV, or adding a DCR.
Of course, if you add a DCR with a different intended DSM layout than
the DSM-example-interface the driver will need to add support for
handling that case.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Linux-nvdimm] [PATCH 04/21] nd: create an 'nd_bus' from an 'nfit_desc'
On Wed, Apr 22, 2015 at 11:23 AM, Toshi Kani toshi.k...@hp.com wrote:
On Wed, 2015-04-22 at 11:20 -0700, Dan Williams wrote:
On Wed, Apr 22, 2015 at 11:00 AM, Linda Knippers linda.knipp...@hp.com
wrote:
Wait, point of clarification, DCRs (dimm-control-regions) have RFICs,
not MEMDEVs (memory-device-to-spa-mapping). Toshi's original report
was that an NFIT with a SPA+MEMDEV was failing to enable a PMEM
device. That specific problem can be fixed by either deleting the
MEMDEV, or adding a DCR.
By a DCR, do you mean a DCR structure or SPA with Control Region GUID?
Hmm, I meant a DCR as defined below. I agree you would not need a SPA-DCR.
Adding a DCR structure does not solve this issue since it requires SPA
with Control Region GUID, which battery-backed DIMMs do not have.
I would not go that far, half of a DCR entry is relevant for any
NVDIMM, and half is only relevant if a DIMM offers BLK access:
struct acpi_nfit_dcr {
u16 type;
u16 length;
u16 dcr_index;
u16 vendor_id;
u16 device_id;
u16 revision_id;
u16 sub_vendor_id;
u16 sub_device_id;
u16 sub_revision_id;
u8 reserved[6];
u32 serial_number;
u16 fic;
BLK relevant fields start here
u16 num_bcw;
u64 bcw_size;
u64 cmd_offset;
u64 cmd_size;
u64 status_offset;
u64 status_size;
u16 flags;
u8 reserved2[6];
};
Of course, if you add a DCR with a different intended DSM layout than
the DSM-example-interface the driver will need to add support for
handling that case.
Yes, we consider to add different _DSMs for management. We do not need
the nd_acpi driver to support it now, but we need this framework to work
without the DSM-example-interface present.
One possible workaround is that I could ignore MEMDEV entries that do
not have a corresponding DCR. This would enable nd_namespace_io
devices to be surfaced for your use case. Would that work for you?
I.e. do you need the nfit_handle exposed?
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Linux-nvdimm] [PATCH 04/21] nd: create an 'nd_bus' from an 'nfit_desc'
On Wed, 2015-04-22 at 12:28 -0700, Dan Williams wrote:
On Wed, Apr 22, 2015 at 11:23 AM, Toshi Kani toshi.k...@hp.com wrote:
On Wed, 2015-04-22 at 11:20 -0700, Dan Williams wrote:
On Wed, Apr 22, 2015 at 11:00 AM, Linda Knippers linda.knipp...@hp.com
wrote:
Wait, point of clarification, DCRs (dimm-control-regions) have RFICs,
not MEMDEVs (memory-device-to-spa-mapping). Toshi's original report
was that an NFIT with a SPA+MEMDEV was failing to enable a PMEM
device. That specific problem can be fixed by either deleting the
MEMDEV, or adding a DCR.
By a DCR, do you mean a DCR structure or SPA with Control Region GUID?
Hmm, I meant a DCR as defined below. I agree you would not need a SPA-DCR.
Adding a DCR structure does not solve this issue since it requires SPA
with Control Region GUID, which battery-backed DIMMs do not have.
I would not go that far, half of a DCR entry is relevant for any
NVDIMM, and half is only relevant if a DIMM offers BLK access:
struct acpi_nfit_dcr {
u16 type;
u16 length;
u16 dcr_index;
u16 vendor_id;
u16 device_id;
u16 revision_id;
u16 sub_vendor_id;
u16 sub_device_id;
u16 sub_revision_id;
u8 reserved[6];
u32 serial_number;
u16 fic;
BLK relevant fields start here
u16 num_bcw;
u64 bcw_size;
u64 cmd_offset;
u64 cmd_size;
u64 status_offset;
u64 status_size;
u16 flags;
u8 reserved2[6];
};
Yes, we do have a DCR entry. But we do not have a SPA-DCR.
The previous issue I reported to nd_mem_init() was caused by the fact
that there was no SPA-DCR. nd_mem_init() requires SPA-DCR to
initialize nd_mem objects.
Of course, if you add a DCR with a different intended DSM layout than
the DSM-example-interface the driver will need to add support for
handling that case.
Yes, we consider to add different _DSMs for management. We do not need
the nd_acpi driver to support it now, but we need this framework to work
without the DSM-example-interface present.
One possible workaround is that I could ignore MEMDEV entries that do
not have a corresponding DCR. This would enable nd_namespace_io
devices to be surfaced for your use case. Would that work for you?
I.e. do you need the nfit_handle exposed?
We have MEMDEV entries and their corresponding DCR entries. ACPI 6.0
states that NVDIMM control region structure index must contain a
non-zero value in a MEMDEV entry, so I think they must correspond.
Yes, we need this framework to enumerate all entries.
Thanks,
-Toshi
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Linux-nvdimm] [PATCH 04/21] nd: create an 'nd_bus' from an 'nfit_desc'
On Wed, Apr 22, 2015 at 12:38 PM, Toshi Kani toshi.k...@hp.com wrote:
On Wed, 2015-04-22 at 12:28 -0700, Dan Williams wrote:
On Wed, Apr 22, 2015 at 11:23 AM, Toshi Kani toshi.k...@hp.com wrote:
On Wed, 2015-04-22 at 11:20 -0700, Dan Williams wrote:
On Wed, Apr 22, 2015 at 11:00 AM, Linda Knippers linda.knipp...@hp.com
wrote:
Wait, point of clarification, DCRs (dimm-control-regions) have RFICs,
not MEMDEVs (memory-device-to-spa-mapping). Toshi's original report
was that an NFIT with a SPA+MEMDEV was failing to enable a PMEM
device. That specific problem can be fixed by either deleting the
MEMDEV, or adding a DCR.
By a DCR, do you mean a DCR structure or SPA with Control Region GUID?
Hmm, I meant a DCR as defined below. I agree you would not need a SPA-DCR.
Adding a DCR structure does not solve this issue since it requires SPA
with Control Region GUID, which battery-backed DIMMs do not have.
I would not go that far, half of a DCR entry is relevant for any
NVDIMM, and half is only relevant if a DIMM offers BLK access:
struct acpi_nfit_dcr {
u16 type;
u16 length;
u16 dcr_index;
u16 vendor_id;
u16 device_id;
u16 revision_id;
u16 sub_vendor_id;
u16 sub_device_id;
u16 sub_revision_id;
u8 reserved[6];
u32 serial_number;
u16 fic;
BLK relevant fields start here
u16 num_bcw;
u64 bcw_size;
u64 cmd_offset;
u64 cmd_size;
u64 status_offset;
u64 status_size;
u16 flags;
u8 reserved2[6];
};
Yes, we do have a DCR entry. But we do not have a SPA-DCR.
Got it. will fix.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Linux-nvdimm] [PATCH 04/21] nd: create an 'nd_bus' from an 'nfit_desc'
On Tue, 2015-04-21 at 13:35 -0700, Dan Williams wrote:
> On Tue, Apr 21, 2015 at 12:55 PM, Toshi Kani wrote:
> > On Tue, 2015-04-21 at 12:58 -0700, Dan Williams wrote:
> >> On Tue, Apr 21, 2015 at 12:35 PM, Toshi Kani wrote:
> >> > On Fri, 2015-04-17 at 21:35 -0400, Dan Williams wrote:
> >> > :
> >> >> +
> >> >> +static int nd_mem_init(struct nd_bus *nd_bus)
> >> >> +{
> >> >> + struct nd_spa *nd_spa;
> >> >> +
> >> >> + /*
> >> >> + * For each SPA-DCR address range find its corresponding
> >> >> + * MEMDEV(s). From each MEMDEV find the corresponding DCR.
> >> >> + * Then, try to find a SPA-BDW and a corresponding BDW that
> >> >> + * references the DCR. Throw it all into an nd_mem object.
> >> >> + * Note, that BDWs are optional.
> >> >> + */
> >> >> + list_for_each_entry(nd_spa, _bus->spas, list) {
> >> >> + u16 spa_index = readw(_spa->nfit_spa->spa_index);
> >> >> + int type = nfit_spa_type(nd_spa->nfit_spa);
> >> >> + struct nd_mem *nd_mem, *found;
> >> >> + struct nd_memdev *nd_memdev;
> >> >> + u16 dcr_index;
> >> >> +
> >> >> + if (type != NFIT_SPA_DCR)
> >> >> + continue;
> >> >
> >> > This function requires NFIT_SPA_DCR, SPA Range Structure with NVDIMM
> >> > Control Region GUID, for initializing an nd_mem object. However,
> >> > battery-backed DIMMs do not have such control region SPA. IIUC, the
> >> > NFIT spec does not require NFIT_SPA_DCR.
> >> >
> >> > Can you change this function to work with NFIT_SPA_PM as well?
> >>
> >> NFIT_SPA_PM ranges are handled separately from nd_mem_init(). See
> >> nd_region_create() in patch 10.
> >
> > If nd_mem_init() does not initialize nd_mem objects, nd_bus_probe() in
> > core.c fails in nd_bus_init_interleave_sets() and skips all subsequent
> > nd_bus_xxx() calls. So, nd_region_create() won't be called.
> >
> > nd_bus_init_interleave_sets() fails because init_interleave_set()
> > returns -ENODEV if (!nd_mem).
>
> Ah, ok your test case is specifying PMEM backed by memory device
> info. We have a test case for simple ranges (nfit_test1_setup()), but
> it doesn't hit this bug because it does not specify any memory-device
> tables.
>
> Thanks, will fix this in v2 of the patch set.
>
> > BTW, there are two nd_bus_probe() in bus.c and core.c, which is
> > confusing.
>
> Ok, will fix this as well in the v2 posting.
Cool! Thanks Dan!
-Toshi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Linux-nvdimm] [PATCH 04/21] nd: create an 'nd_bus' from an 'nfit_desc'
On Tue, Apr 21, 2015 at 12:55 PM, Toshi Kani wrote:
> On Tue, 2015-04-21 at 12:58 -0700, Dan Williams wrote:
>> On Tue, Apr 21, 2015 at 12:35 PM, Toshi Kani wrote:
>> > On Fri, 2015-04-17 at 21:35 -0400, Dan Williams wrote:
>> > :
>> >> +
>> >> +static int nd_mem_init(struct nd_bus *nd_bus)
>> >> +{
>> >> + struct nd_spa *nd_spa;
>> >> +
>> >> + /*
>> >> + * For each SPA-DCR address range find its corresponding
>> >> + * MEMDEV(s). From each MEMDEV find the corresponding DCR.
>> >> + * Then, try to find a SPA-BDW and a corresponding BDW that
>> >> + * references the DCR. Throw it all into an nd_mem object.
>> >> + * Note, that BDWs are optional.
>> >> + */
>> >> + list_for_each_entry(nd_spa, _bus->spas, list) {
>> >> + u16 spa_index = readw(_spa->nfit_spa->spa_index);
>> >> + int type = nfit_spa_type(nd_spa->nfit_spa);
>> >> + struct nd_mem *nd_mem, *found;
>> >> + struct nd_memdev *nd_memdev;
>> >> + u16 dcr_index;
>> >> +
>> >> + if (type != NFIT_SPA_DCR)
>> >> + continue;
>> >
>> > This function requires NFIT_SPA_DCR, SPA Range Structure with NVDIMM
>> > Control Region GUID, for initializing an nd_mem object. However,
>> > battery-backed DIMMs do not have such control region SPA. IIUC, the
>> > NFIT spec does not require NFIT_SPA_DCR.
>> >
>> > Can you change this function to work with NFIT_SPA_PM as well?
>>
>> NFIT_SPA_PM ranges are handled separately from nd_mem_init(). See
>> nd_region_create() in patch 10.
>
> If nd_mem_init() does not initialize nd_mem objects, nd_bus_probe() in
> core.c fails in nd_bus_init_interleave_sets() and skips all subsequent
> nd_bus_xxx() calls. So, nd_region_create() won't be called.
>
> nd_bus_init_interleave_sets() fails because init_interleave_set()
> returns -ENODEV if (!nd_mem).
Ah, ok your test case is specifying PMEM backed by memory device
info. We have a test case for simple ranges (nfit_test1_setup()), but
it doesn't hit this bug because it does not specify any memory-device
tables.
Thanks, will fix this in v2 of the patch set.
> BTW, there are two nd_bus_probe() in bus.c and core.c, which is
> confusing.
Ok, will fix this as well in the v2 posting.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Linux-nvdimm] [PATCH 04/21] nd: create an 'nd_bus' from an 'nfit_desc'
On Tue, 2015-04-21 at 12:58 -0700, Dan Williams wrote:
> On Tue, Apr 21, 2015 at 12:35 PM, Toshi Kani wrote:
> > On Fri, 2015-04-17 at 21:35 -0400, Dan Williams wrote:
> > :
> >> +
> >> +static int nd_mem_init(struct nd_bus *nd_bus)
> >> +{
> >> + struct nd_spa *nd_spa;
> >> +
> >> + /*
> >> + * For each SPA-DCR address range find its corresponding
> >> + * MEMDEV(s). From each MEMDEV find the corresponding DCR.
> >> + * Then, try to find a SPA-BDW and a corresponding BDW that
> >> + * references the DCR. Throw it all into an nd_mem object.
> >> + * Note, that BDWs are optional.
> >> + */
> >> + list_for_each_entry(nd_spa, _bus->spas, list) {
> >> + u16 spa_index = readw(_spa->nfit_spa->spa_index);
> >> + int type = nfit_spa_type(nd_spa->nfit_spa);
> >> + struct nd_mem *nd_mem, *found;
> >> + struct nd_memdev *nd_memdev;
> >> + u16 dcr_index;
> >> +
> >> + if (type != NFIT_SPA_DCR)
> >> + continue;
> >
> > This function requires NFIT_SPA_DCR, SPA Range Structure with NVDIMM
> > Control Region GUID, for initializing an nd_mem object. However,
> > battery-backed DIMMs do not have such control region SPA. IIUC, the
> > NFIT spec does not require NFIT_SPA_DCR.
> >
> > Can you change this function to work with NFIT_SPA_PM as well?
>
> NFIT_SPA_PM ranges are handled separately from nd_mem_init(). See
> nd_region_create() in patch 10.
If nd_mem_init() does not initialize nd_mem objects, nd_bus_probe() in
core.c fails in nd_bus_init_interleave_sets() and skips all subsequent
nd_bus_xxx() calls. So, nd_region_create() won't be called.
nd_bus_init_interleave_sets() fails because init_interleave_set()
returns -ENODEV if (!nd_mem).
BTW, there are two nd_bus_probe() in bus.c and core.c, which is
confusing.
Thanks,
-Toshi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Linux-nvdimm] [PATCH 04/21] nd: create an 'nd_bus' from an 'nfit_desc'
On Tue, Apr 21, 2015 at 12:35 PM, Toshi Kani wrote:
> On Fri, 2015-04-17 at 21:35 -0400, Dan Williams wrote:
> :
>> +
>> +static int nd_mem_init(struct nd_bus *nd_bus)
>> +{
>> + struct nd_spa *nd_spa;
>> +
>> + /*
>> + * For each SPA-DCR address range find its corresponding
>> + * MEMDEV(s). From each MEMDEV find the corresponding DCR.
>> + * Then, try to find a SPA-BDW and a corresponding BDW that
>> + * references the DCR. Throw it all into an nd_mem object.
>> + * Note, that BDWs are optional.
>> + */
>> + list_for_each_entry(nd_spa, _bus->spas, list) {
>> + u16 spa_index = readw(_spa->nfit_spa->spa_index);
>> + int type = nfit_spa_type(nd_spa->nfit_spa);
>> + struct nd_mem *nd_mem, *found;
>> + struct nd_memdev *nd_memdev;
>> + u16 dcr_index;
>> +
>> + if (type != NFIT_SPA_DCR)
>> + continue;
>
> This function requires NFIT_SPA_DCR, SPA Range Structure with NVDIMM
> Control Region GUID, for initializing an nd_mem object. However,
> battery-backed DIMMs do not have such control region SPA. IIUC, the
> NFIT spec does not require NFIT_SPA_DCR.
>
> Can you change this function to work with NFIT_SPA_PM as well?
NFIT_SPA_PM ranges are handled separately from nd_mem_init(). See
nd_region_create() in patch 10.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Linux-nvdimm] [PATCH 04/21] nd: create an 'nd_bus' from an 'nfit_desc'
On Fri, 2015-04-17 at 21:35 -0400, Dan Williams wrote:
:
> +
> +static int nd_mem_init(struct nd_bus *nd_bus)
> +{
> + struct nd_spa *nd_spa;
> +
> + /*
> + * For each SPA-DCR address range find its corresponding
> + * MEMDEV(s). From each MEMDEV find the corresponding DCR.
> + * Then, try to find a SPA-BDW and a corresponding BDW that
> + * references the DCR. Throw it all into an nd_mem object.
> + * Note, that BDWs are optional.
> + */
> + list_for_each_entry(nd_spa, _bus->spas, list) {
> + u16 spa_index = readw(_spa->nfit_spa->spa_index);
> + int type = nfit_spa_type(nd_spa->nfit_spa);
> + struct nd_mem *nd_mem, *found;
> + struct nd_memdev *nd_memdev;
> + u16 dcr_index;
> +
> + if (type != NFIT_SPA_DCR)
> + continue;
This function requires NFIT_SPA_DCR, SPA Range Structure with NVDIMM
Control Region GUID, for initializing an nd_mem object. However,
battery-backed DIMMs do not have such control region SPA. IIUC, the
NFIT spec does not require NFIT_SPA_DCR.
Can you change this function to work with NFIT_SPA_PM as well?
Thanks,
-Toshi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Linux-nvdimm] [PATCH 04/21] nd: create an 'nd_bus' from an 'nfit_desc'
On Fri, 2015-04-17 at 21:35 -0400, Dan Williams wrote:
:
+
+static int nd_mem_init(struct nd_bus *nd_bus)
+{
+ struct nd_spa *nd_spa;
+
+ /*
+ * For each SPA-DCR address range find its corresponding
+ * MEMDEV(s). From each MEMDEV find the corresponding DCR.
+ * Then, try to find a SPA-BDW and a corresponding BDW that
+ * references the DCR. Throw it all into an nd_mem object.
+ * Note, that BDWs are optional.
+ */
+ list_for_each_entry(nd_spa, nd_bus-spas, list) {
+ u16 spa_index = readw(nd_spa-nfit_spa-spa_index);
+ int type = nfit_spa_type(nd_spa-nfit_spa);
+ struct nd_mem *nd_mem, *found;
+ struct nd_memdev *nd_memdev;
+ u16 dcr_index;
+
+ if (type != NFIT_SPA_DCR)
+ continue;
This function requires NFIT_SPA_DCR, SPA Range Structure with NVDIMM
Control Region GUID, for initializing an nd_mem object. However,
battery-backed DIMMs do not have such control region SPA. IIUC, the
NFIT spec does not require NFIT_SPA_DCR.
Can you change this function to work with NFIT_SPA_PM as well?
Thanks,
-Toshi
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Linux-nvdimm] [PATCH 04/21] nd: create an 'nd_bus' from an 'nfit_desc'
On Tue, 2015-04-21 at 12:58 -0700, Dan Williams wrote:
On Tue, Apr 21, 2015 at 12:35 PM, Toshi Kani toshi.k...@hp.com wrote:
On Fri, 2015-04-17 at 21:35 -0400, Dan Williams wrote:
:
+
+static int nd_mem_init(struct nd_bus *nd_bus)
+{
+ struct nd_spa *nd_spa;
+
+ /*
+ * For each SPA-DCR address range find its corresponding
+ * MEMDEV(s). From each MEMDEV find the corresponding DCR.
+ * Then, try to find a SPA-BDW and a corresponding BDW that
+ * references the DCR. Throw it all into an nd_mem object.
+ * Note, that BDWs are optional.
+ */
+ list_for_each_entry(nd_spa, nd_bus-spas, list) {
+ u16 spa_index = readw(nd_spa-nfit_spa-spa_index);
+ int type = nfit_spa_type(nd_spa-nfit_spa);
+ struct nd_mem *nd_mem, *found;
+ struct nd_memdev *nd_memdev;
+ u16 dcr_index;
+
+ if (type != NFIT_SPA_DCR)
+ continue;
This function requires NFIT_SPA_DCR, SPA Range Structure with NVDIMM
Control Region GUID, for initializing an nd_mem object. However,
battery-backed DIMMs do not have such control region SPA. IIUC, the
NFIT spec does not require NFIT_SPA_DCR.
Can you change this function to work with NFIT_SPA_PM as well?
NFIT_SPA_PM ranges are handled separately from nd_mem_init(). See
nd_region_create() in patch 10.
If nd_mem_init() does not initialize nd_mem objects, nd_bus_probe() in
core.c fails in nd_bus_init_interleave_sets() and skips all subsequent
nd_bus_xxx() calls. So, nd_region_create() won't be called.
nd_bus_init_interleave_sets() fails because init_interleave_set()
returns -ENODEV if (!nd_mem).
BTW, there are two nd_bus_probe() in bus.c and core.c, which is
confusing.
Thanks,
-Toshi
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Linux-nvdimm] [PATCH 04/21] nd: create an 'nd_bus' from an 'nfit_desc'
On Tue, Apr 21, 2015 at 12:55 PM, Toshi Kani toshi.k...@hp.com wrote:
On Tue, 2015-04-21 at 12:58 -0700, Dan Williams wrote:
On Tue, Apr 21, 2015 at 12:35 PM, Toshi Kani toshi.k...@hp.com wrote:
On Fri, 2015-04-17 at 21:35 -0400, Dan Williams wrote:
:
+
+static int nd_mem_init(struct nd_bus *nd_bus)
+{
+ struct nd_spa *nd_spa;
+
+ /*
+ * For each SPA-DCR address range find its corresponding
+ * MEMDEV(s). From each MEMDEV find the corresponding DCR.
+ * Then, try to find a SPA-BDW and a corresponding BDW that
+ * references the DCR. Throw it all into an nd_mem object.
+ * Note, that BDWs are optional.
+ */
+ list_for_each_entry(nd_spa, nd_bus-spas, list) {
+ u16 spa_index = readw(nd_spa-nfit_spa-spa_index);
+ int type = nfit_spa_type(nd_spa-nfit_spa);
+ struct nd_mem *nd_mem, *found;
+ struct nd_memdev *nd_memdev;
+ u16 dcr_index;
+
+ if (type != NFIT_SPA_DCR)
+ continue;
This function requires NFIT_SPA_DCR, SPA Range Structure with NVDIMM
Control Region GUID, for initializing an nd_mem object. However,
battery-backed DIMMs do not have such control region SPA. IIUC, the
NFIT spec does not require NFIT_SPA_DCR.
Can you change this function to work with NFIT_SPA_PM as well?
NFIT_SPA_PM ranges are handled separately from nd_mem_init(). See
nd_region_create() in patch 10.
If nd_mem_init() does not initialize nd_mem objects, nd_bus_probe() in
core.c fails in nd_bus_init_interleave_sets() and skips all subsequent
nd_bus_xxx() calls. So, nd_region_create() won't be called.
nd_bus_init_interleave_sets() fails because init_interleave_set()
returns -ENODEV if (!nd_mem).
Ah, ok your test case is specifying PMEM backed by memory device
info. We have a test case for simple ranges (nfit_test1_setup()), but
it doesn't hit this bug because it does not specify any memory-device
tables.
Thanks, will fix this in v2 of the patch set.
BTW, there are two nd_bus_probe() in bus.c and core.c, which is
confusing.
Ok, will fix this as well in the v2 posting.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Linux-nvdimm] [PATCH 04/21] nd: create an 'nd_bus' from an 'nfit_desc'
On Tue, Apr 21, 2015 at 12:35 PM, Toshi Kani toshi.k...@hp.com wrote:
On Fri, 2015-04-17 at 21:35 -0400, Dan Williams wrote:
:
+
+static int nd_mem_init(struct nd_bus *nd_bus)
+{
+ struct nd_spa *nd_spa;
+
+ /*
+ * For each SPA-DCR address range find its corresponding
+ * MEMDEV(s). From each MEMDEV find the corresponding DCR.
+ * Then, try to find a SPA-BDW and a corresponding BDW that
+ * references the DCR. Throw it all into an nd_mem object.
+ * Note, that BDWs are optional.
+ */
+ list_for_each_entry(nd_spa, nd_bus-spas, list) {
+ u16 spa_index = readw(nd_spa-nfit_spa-spa_index);
+ int type = nfit_spa_type(nd_spa-nfit_spa);
+ struct nd_mem *nd_mem, *found;
+ struct nd_memdev *nd_memdev;
+ u16 dcr_index;
+
+ if (type != NFIT_SPA_DCR)
+ continue;
This function requires NFIT_SPA_DCR, SPA Range Structure with NVDIMM
Control Region GUID, for initializing an nd_mem object. However,
battery-backed DIMMs do not have such control region SPA. IIUC, the
NFIT spec does not require NFIT_SPA_DCR.
Can you change this function to work with NFIT_SPA_PM as well?
NFIT_SPA_PM ranges are handled separately from nd_mem_init(). See
nd_region_create() in patch 10.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Linux-nvdimm] [PATCH 04/21] nd: create an 'nd_bus' from an 'nfit_desc'
On Tue, 2015-04-21 at 13:35 -0700, Dan Williams wrote:
On Tue, Apr 21, 2015 at 12:55 PM, Toshi Kani toshi.k...@hp.com wrote:
On Tue, 2015-04-21 at 12:58 -0700, Dan Williams wrote:
On Tue, Apr 21, 2015 at 12:35 PM, Toshi Kani toshi.k...@hp.com wrote:
On Fri, 2015-04-17 at 21:35 -0400, Dan Williams wrote:
:
+
+static int nd_mem_init(struct nd_bus *nd_bus)
+{
+ struct nd_spa *nd_spa;
+
+ /*
+ * For each SPA-DCR address range find its corresponding
+ * MEMDEV(s). From each MEMDEV find the corresponding DCR.
+ * Then, try to find a SPA-BDW and a corresponding BDW that
+ * references the DCR. Throw it all into an nd_mem object.
+ * Note, that BDWs are optional.
+ */
+ list_for_each_entry(nd_spa, nd_bus-spas, list) {
+ u16 spa_index = readw(nd_spa-nfit_spa-spa_index);
+ int type = nfit_spa_type(nd_spa-nfit_spa);
+ struct nd_mem *nd_mem, *found;
+ struct nd_memdev *nd_memdev;
+ u16 dcr_index;
+
+ if (type != NFIT_SPA_DCR)
+ continue;
This function requires NFIT_SPA_DCR, SPA Range Structure with NVDIMM
Control Region GUID, for initializing an nd_mem object. However,
battery-backed DIMMs do not have such control region SPA. IIUC, the
NFIT spec does not require NFIT_SPA_DCR.
Can you change this function to work with NFIT_SPA_PM as well?
NFIT_SPA_PM ranges are handled separately from nd_mem_init(). See
nd_region_create() in patch 10.
If nd_mem_init() does not initialize nd_mem objects, nd_bus_probe() in
core.c fails in nd_bus_init_interleave_sets() and skips all subsequent
nd_bus_xxx() calls. So, nd_region_create() won't be called.
nd_bus_init_interleave_sets() fails because init_interleave_set()
returns -ENODEV if (!nd_mem).
Ah, ok your test case is specifying PMEM backed by memory device
info. We have a test case for simple ranges (nfit_test1_setup()), but
it doesn't hit this bug because it does not specify any memory-device
tables.
Thanks, will fix this in v2 of the patch set.
BTW, there are two nd_bus_probe() in bus.c and core.c, which is
confusing.
Ok, will fix this as well in the v2 posting.
Cool! Thanks Dan!
-Toshi
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
