Thread (10 messages) 10 messages, 2 authors, 2014-11-25

Re: [PATCH 1/5] imsm: support for OROMs shared by multiple HBAs

From: NeilBrown <hidden>
Date: 2014-11-25 00:51:54

On Thu, 20 Nov 2014 18:50:03 +0100 Artur Paszkiewicz
[off-list ref] wrote:
On 11/20/2014 04:07 AM, NeilBrown wrote:
quoted
On Wed, 19 Nov 2014 13:53:26 +0100 Artur Paszkiewicz
[off-list ref] wrote:
quoted
HBAs can share OROMs (e.g. SATA/sSATA). They are matched by PCI device
id. Removed populated_orom/efi and imsm_orom/efi arrays - they are
replaced by oroms array and functions get_orom_by_device_id(),
add_orom(), add_orom_device_id().

Signed-off-by: Artur Paszkiewicz <redacted>
Hi,
 this patch seems to make a lot more changes that the above brief description
 seems to suggest.
 Is there any chance of breaking it up into two or three parts, or at least
 describing everything that is being changed.

 I'm half tempted to just accept it as it is, as it is just "your" code that
 that is being changed, but I'd like to understand it if I can.

Thanks,
NeilBrown
Hi Neil,

Splitting this up reasonably turned out to be more difficult than I
thought, so I'll try to provide a more detailed description of the
changes. 

The IMSM platform code was based on an assumption that the OROM or UEFI
capability structure (represented by struct imsm_orom) always belongs to
only one HBA. This assumption is no longer valid, because of newer
platforms with dual AHCI HBAs. Each HBA can have a separate OROM, but
some versions have a combined OROM for both HBAs.

This patch implements this HBA-OROM relationship in struct orom_entry,
which matches an OROM with a list of HBA PCI ids. All the detected
orom_entries are stored and retrieved using a global array and the
functions add_orom(), add_orom_device_id() and get_orom_by_device_id().
This replaces the arrays: imsm_orom, populated_orom, imsm_efi,
populated_efi.

The scan() function is extended to find all HBAs for an OROM. The list
of their device ids is retrieved from the PCI Expansion ROM Data
Structure, hence the additional field devListOffset in struct
pciExpDataStructFormat.

In UEFI mode we can't read the PCI Expansion ROM Data Structure and the
imsm_orom structures are stored in UEFI variables. They do not provide a
similar device id list, so we also check the HBA PCI class to make sure
that the HBA has RAID mode enabled.

In super-intel.c there are changes which allow spanning of IMSM
containers over HBAs of the same type, but only if the HBAs share the
same OROM.  This is done by comparing imsm_orom pointers, which (outside
of platform-intel.c) always point to the global array containing all the
detected oroms. Additional warnings are added to
validate_container_imsm() to warn about potentially dangerous operations
in all the possible cases, e.g. when an array is assembled using disks
attached to HBAs with separate OROMs.

I hope you find this description helpful and that it will make the
changes easier to understand.
Much better - thanks!

I've applied all 5 patches - using v2 of patch 4, and this comment for patch
1.

Thanks,
NeilBrown

Attachments

  • (unnamed) [application/pgp-signature] 811 bytes
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help