On Tue, Nov 14, 2017 at 06:44:51PM -0200, Paulo Zanoni wrote: > Em Ter, 2017-11-14 às 22:34 +0200, Ville Syrjälä escreveu: > > On Tue, Nov 14, 2017 at 06:12:41PM -0200, Paulo Zanoni wrote: > > > Em Qui, 2017-11-02 às 17:17 +0200, Ville Syrjala escreveu: > > > > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > > > > > > Apparently there are some machines that put semi-sensible looking > > > > values > > > > into the stolen "reserved" base and size, except those values are > > > > actually > > > > outside the stolen memory. There is a bit in the register which > > > > supposedly could tell us whether the reserved area is even > > > > enabled or > > > > not. Let's check for that before we go trusting the base and > > > > size. > > > > > > If this is such a problem since g4x, why didn't we spot it earlier? > > > It > > > would be nice if you could explain in the commit message (or at > > > least > > > in this email) what are the consequences you're seeing that made > > > you > > > realize about this problem. Did something actually explode? I'm > > > genuinely curious. > > > > CI is getting ping-ping on the SNB shards. Apparently the BIOS leaves > > some garbage in the the base+size fields of the register, and > > sometimes > > that results in the reserved area landing inside stolen (at which > > point > > we just reduce the size of stolen and continue), and sometimes it > > lands > > somewhere outside (at which point we just disable stolen entirely). > > Hence the FBC tests sometimes find enough stolen, sometimes not. > > Couldn't it be that we're failing to read the "size of stolen" > registers instead of the "stolen reserved stuff" regs? All of the > stolen registers have a history of controversy... I think stolen itself looks correct. IIRC it was somewhere above the 3 GiB mark and neatly inside a e820 reserved region. The reserved region (at least in the logs I looked at) was somewhere around the 17 MiB mark, right on top of normal RAM according to e820. No sane BIOS would put it there. > > > > > > > > > > > > > Now talking about the patch itself: > > > > > > If we're going to assume random bits instead of a full-zero in > > > (possibly) uninitialized stuff, shouldn't we first check bit 1, > > > which > > > is supposed to tell us if the whole reg is locked or not? > > > > Not sure we can trust the BIOS to always set the lock bit. I suppose > > it > > really should, but I don't recall seeing anything in the docs saying > > that not setting the lock bit would somehow disable the reserved > > area. > > The way things are worded, it suggests that if the lock bit is zero, > everything else could be garbage. Bit 0 is only locked after bit 1 is > locked. It doesn't make sense to me to trust bit 0 without trusting bit > 1. The hardware itself will trust bit 0 without bit 1. I think that's all that really matters. > > So my understanding is that as long as the enable bit is set the > > hardware will prevent us from accessing the are, regardless of > > whether > > the settings have been locked down or not. > > > > This wouldn't be the first lock bit some BIOS versions forget to set > > BTW. > > But it could be worth checking this now that you have access to the > machines that have the random stuff... Yeah. I guess it might be interesting to see the state of the lock bit as well. I'll see if I can get someone to grab it for us (/me no longer has any idea how to access those machines...). -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
