Date: Mon, 14 Feb 2011 14:26:36 -0500 From: Jung-uk Kim <jkim@FreeBSD.org> To: Matthew Fleming <mdf356@gmail.com> Cc: freebsd-current@freebsd.org Subject: Re: acpi_resource bug? Message-ID: <201102141426.42450.jkim@FreeBSD.org> In-Reply-To: <AANLkTimDh_s0UTXkKqBeXDjJiub3LYqL8oR0pms5PoCP@mail.gmail.com> References: <AANLkTi=C31iGJMonj7E3DGLWhx0cSKQR=b7ZHTv9CdmA@mail.gmail.com> <201102141337.59203.jhb@freebsd.org> <AANLkTimDh_s0UTXkKqBeXDjJiub3LYqL8oR0pms5PoCP@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Monday 14 February 2011 01:45 pm, Matthew Fleming wrote: > On Mon, Feb 14, 2011 at 10:37 AM, John Baldwin <jhb@freebsd.org> wrote: > > On Monday, February 14, 2011 1:30:18 pm Jung-uk Kim wrote: > >> On Monday 14 February 2011 10:29 am, Matthew Fleming wrote: > >> > On Mon, Feb 14, 2011 at 6:24 AM, John Baldwin > >> > <jhb@freebsd.org> > >> > >> wrote: > >> > > On Sunday, February 13, 2011 2:46:07 pm Matthew Fleming wrote: > >> > >> I'm not very familiar with the acpi code, but we have seen > >> > >> an intermittent issue on boot: > >> > >> > >> > >> 1) should the length of the bcopy() be changed to either > >> > >> respect res->Length or the actual length of the > >> > >> ACPI_RESOURCE_DATA for the type? > >> > > > >> > > It should just use res->Length: > >> > > >> > Is there a guarantee that res->Length is <= > >> > sizeof(ACPI_RESOURCE) ? > >> > >> No. �Please try the attached patch (after your r218685). > > > > I think your patch is correct, but are you saying that ACPICA > > will return a resource with a size that doesn't match its type? > > > > ACPI_RESOURCE_DATA is a union of all the various resource types, > > and it does contain both ACPI_RESOURCE_IRQ and > > ACPI_RESOURCE_EXTENDED_IRQ, so it's hard to see how res->Length > > would be greater than the size of ACPI_RESOURCE. > > Jung-uk Kim's patch makes acpi_resource match the other bcopy's in > the acpica directory, and in the case of what I saw it would bcopy > 8+5 bytes instead of res->Length == 16. Actually, it is 8+6 bytes, i.e., sizeof(ACPI_RESOURCE_IRQ) == 6. ;-) Also, res->Length does not include header but it includes padding bytes for alignment. > My concern was that res->Length seemed primarily to be an offset > from the current resource to the next one, and I didn't see why > that may not include a lot of padding (including more than the > target of the bcopy was prepared for). However, my code will also > copy bytes we don't care about any time res->Length is rounded up > from the actual struct size. Correct. > The patch looks fine to me. I don't have direct access to the > machine that was intermittently crashing so I can't really try the > new patch, but my change resolved the issue on it. I'll go ahead and commit my patch if you don't have any more concerns. Thanks for working on this, Jung-uk Kim
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201102141426.42450.jkim>