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

[NeilBrown <neilb@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 3221 bytes --]

On Thu, 20 Nov 2014 18:50:03 +0100 Artur Paszkiewicz
<artur.paszkiewicz@intel.com> wrote:

> On 11/20/2014 04:07 AM, NeilBrown wrote:
> > On Wed, 19 Nov 2014 13:53:26 +0100 Artur Paszkiewicz
> > <artur.paszkiewicz@intel.com> wrote:
> > 
> >> 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 <artur.paszkiewicz@intel.com>
> > 
> > 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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]