Thinkpad, Thinkpad, Thinkpad

[Peter Jones]

On Tue, 2006-03-28 at 16:29 -0500, Alan Cox wrote:
> On Tue, Mar 28, 2006 at 02:54:17PM -0500, Peter Jones wrote:
> > Speaking of which, I've got a patch that duplicates our current startup
> > breakage in the resume path.  Which is to say, since we've already
> > screwed up the data that's in the protected area, this just disables it
> > again during resume, so you don't get IO errors.
> 
> Its not breakage its quite intentional. For 99% of cases HPA is set because
> it is being used fo BIOS clipping

For all thinkpads that have this turned on it's for BIOS backup/restore
features, a and other related features that the user (at least
theoretically) paid for and may still want.  Right now we're trashing
them.

I think ThinkPads make up more than 1% of cases, but I've got no real
data to prove it with.

> 
> > http://people.redhat.com/pjones/hpa/linux-2.6.16-hpa-resume.patch
> 
> Looks basically sane. You have to run the ACPI taskfiles from the BIOS first
> however or you are out of spec and anything can happen (although fortunately
> it ususally doesnt)

Ugh.  Ok, I've got some reading to do before I get this in.  Thanks for
the info.

> Problems in the patch aside from that
> 
> #1:	HPA support and active state are in the identify data so you should
> 	consult that (which is boot time cached).

drive->capacity64 comes from one of id->lba_capacity_2,
id->lba_capacity, or CHS .  Is there something else you mean I should be
consulting the identify data for?

> #2:	You can't assume LBA48 read_native_max_address_ext is supported just
> 	because LBA48 is, some maxtors don't. (dont copy the base kernel HPA
> 	code that is broken too but fails fairly safe with just message spew)

Oh, you're absolutely right here, there's should be a check for the hpa
featureset present there.  Thanks, I'll update that.

> #3:	The proper way to decide to issue a set max is to check if the
> 	HPA is active but the size check is fine too I think
>
> #4:	You need to tell the block layer about any size changes.

Well, currently that's not a problem -- in the second patch I have the
set_capacity() calls.  But in this patch it's not needed, because we
always disable HPA no matter what before the gendisk is initialized at
all, so the block layer has never known about the smaller size.

> > (obviously, this needs to be done for libata as well)
> 
> At the moment libata doesn't do HPA. Some ideas have been kicked around for
> how to handle it neatly but no conclusions AFAIK reached. Lobby Jeff,
> especially if you have a smart idea.

I was thinking something almost exactly like the interface that's in the
second patch I posted.  We've already shipped 2 releases that just
disable it entirely for IDE, which means if we ever want to do anything
other than just disable it, we need control to be somewhere that can
examine the system to see what we've done.  Sadly, that probably means
userland.

-- 
  Peter