problems with AHCI on FreeBSD 8.2
Victor Balada Diaz
victor at bsdes.net
Wed Feb 15 08:29:11 UTC 2012
On Tue, Feb 14, 2012 at 04:10:20PM -0800, Jeremy Chadwick wrote:
> I'm too tired to quite understand (in full) what's wrong with my patch,
> but I think you're referring to situations where someone would have
> kern.cam.ada.X.quirks set in loader.conf?
> If so, I believe that same situation would happen presently if someone
> set kern.cam.ada.X.quirks in their loader.conf to a value that did not
> contain bit #0 set to 1, and used one of the 4K sector disks listed in
> ada_quirk_table -- what's in loader.conf looks like it would overwrite
> whatever the kernel code bits chose automatically:
> 910 match = cam_quirkmatch((caddr_t)&cgd->ident_data,
> 911 (caddr_t)ada_quirk_table,
> 912 sizeof(ada_quirk_table)/sizeof(*ada_quirk_table),
> 913 sizeof(*ada_quirk_table), ata_identify_match);
> 914 if (match != NULL)
> 915 softc->quirks = ((struct ada_quirk_entry *)match)->quirks;
> 916 else
> 917 softc->quirks = ADA_Q_NONE;
> 931 snprintf(announce_buf, sizeof(announce_buf),
> 932 "kern.cam.ada.%d.quirks", periph->unit_number);
> 933 quirks = softc->quirks;
> 934 TUNABLE_INT_FETCH(announce_buf, &quirks);
> 935 softc->quirks = quirks;
> I read this to mean:
> Lines 910-917 -- if there's a device ID string match in ada_quirk_table,
> set softc->quirks to the content of that struct entry. So, for example,
> 4K sector disks would set softc->quirks to 0x01.
> Lines 931-933 -- assign the "quirks" variable to what softc->quirks
> currently contains. Thus, for 4K sector disks, quirks = 0x01.
> Line 934 -- load into "quirks" variable the contents of loader.conf
> entries pertaining to kern.cam.ada.N.quirks, if set. If someone had an
> entry in loader.conf saying kern.cam.ada.N.quirks=0 then yes, this would
> overwrite what the kernel "auto-chose".
> Line 935 -- assign softc->quirks = quirks, thus softc->quirks = 0x00,
> assuming someone set it to such in loader.conf.
That's exactly what i meant.
> > See my attached patch. I can confirm it works for me.
> I thought of taking that approach, but for me it's "dirty". Here's what
> I mean by that:
> ADA_FLAG_CAN_NCQ gets set in softc->flags around line 892, but then
> roughly a hundred lines later, you clear the exact same flag you just
> set (based on quirk conditionals).
> I dunno how people feel about that, but my impression is that it's
> dirty/confusing. My opinion is to only set the bit once and not mess
> about with repeated if() statements that set it, then clear it, etc...
Indeed you're right. It's a hack. Would be better to move quirk evaluation before checking
La prueba más fehaciente de que existe vida inteligente en otros
planetas, es que no han intentado contactar con nosotros.
More information about the freebsd-stable