drm/nouveau/bios: use size provided by _ROM method
Subsystems (3)
PATCH 1/1 drm/nouveau/bios: use size provided by _ROM method
From: Peter Wu <[email protected]>
Since commit "drm/nouveau/bios: attempt to fetch entire acpi rom image in one shot", the ACPI spec is broken in order to gain speed. In theory, since the _ROM method is supposed to return 4 KiB only, the returned buffer size could be less than the requested length. This could lead to reading past the buffer boundaries which could make worse thing happen. To fix that, do not read more than the buffer contains. As an extra side-effect, the function returns the bytes that have really been read which is more natural.
--- drivers/gpu/drm/nouveau/nouveau_acpi.c | 1 + 1 file changed, 1 insertion(+)
drivers/gpu/drm/nouveau/nouveau_acpi.c +1 -0
@@ -356,6 +356,7 @@ static int nouveau_rom_call(acpi_handle rom_handle, uint8_t *bios,return -ENODEV ; } obj = (union acpi_object *)buffer.pointer; + len =min (len, (int )obj->buffer.size);memcpy (bios+offset, obj->buffer.pointer, len);kfree (buffer.pointer);return len;
Signature · 1 line
1.7.9.5
Quoted 21 lines · Lekensteyn
From: Peter Wu <[email protected]>
Since commit "drm/nouveau/bios: attempt to fetch entire acpi rom image in one shot", the ACPI spec is broken in order to gain speed. In theory, since the
_ROMmethod is supposed to return 4 KiB only, the returned buffer size could be less than the requested length. This could lead to reading past the buffer boundaries which could make worse thing happen. To fix that, do not read more than the buffer contains. As an extra side-effect, the function returns the bytes that have really been read which is more natural.Signed-off-by Peter Wu <[email protected]>--- drivers/gpu/drm/nouveau/nouveau_acpi.c | 1 + 1 file changed, 1 insertion(+)drivers/gpu/drm/nouveau/nouveau_acpi.c +0
-0
drivers/gpu/drm/nouveau/nouveau_acpi.c static int nouveau_rom_call(acpi_handle rom_handle, uint8_t *bios, · line 362
Quoted 7 lines
drivers/gpu/drm/nouveau/nouveau_acpi.c +1
-0@@ -356,6 +356,7 @@ static int nouveau_rom_call(acpi_handle rom_handle, uint8_t *bios,return -ENODEV ; } obj = (union acpi_object *)buffer.pointer; + len =min (len, (int )obj->buffer.size);memcpy (bios+offset, obj->buffer.pointer, len);kfree (buffer.pointer);return len;
BTW, _ROM method from https://lkml.org/lkml/2012/10/21/11 multiplies length by 8, so maybe we can read 32kB in one batch?
Marcin
A message that is not in the archive (sent off-list or not yet received): [email protected]
Quoted 4 lines · On Sunday 21 October 2012 14:20:05 Marcin Slusarz wrote:
BTW,
_ROMmethod from https://lkml.org/lkml/2012/10/21/11 multiplies length by 8, so maybe we can read 32kB in one batch?Marcin
That holds for that specific model, but it is not standard (in ACPI). Ben Skeggs has a laptop (Lenovo?) W530 that allows you to read the whole VBIOS in one call (that means chunk size > 4KiB), but there is another machine where this does not work (https://bugs.freedesktop.org/show_bug.cgi?id=55948). That machine allows you to read lengths of 4 KiB only (the buffer size is hard-coded to 4096, the upper bound of the returned size).
I'd say, stick to 4 KiB for the first read, then try to read the remaining VBIOS in one shot and finally fallback to reading in chunks of min(4096, bios-
Quoted 1 line
size - i). That should give a count of least 512 bytes. On my machine the
length is 0x7d which translates to exactly 64 KB (not KiB).
After your patch in https://lkml.org/lkml/2012/10/20/150, "drm/nouveau/bios:
attempt to fetch entire acpi rom image in one shot" could be improved to only read the unknown VBIOS:
/* disobey the acpi spec - much faster on at least w530 ... */
- ret = nouveau_acpi_get_bios_chunk(bios->data, 0, bios->size);
+ ret = nouveau_acpi_get_bios_chunk(bios->data, 4096, bios->size - 4096);
Peter