On Mon 27 Feb 2017, Jason Ekstrand wrote: > On Mon, Feb 27, 2017 at 9:38 AM, Chad Versace <chadvers...@chromium.org> > wrote: > > > On Fri 24 Feb 2017, Jason Ekstrand wrote: > > > This prevents a user from using a cache created on one hardware > > > generation on a different one. Of course, with Intel hardware, this > > > requires moving their drive from one machine to another but it's still > > > possible and we should prevent it. > > > > Or if you rsync stuff between test machines. > > > > Or if your test machines share stuff over NFS. (Mine do). > > > > Or... > > > > It's easy to hit this bug without a screwdriver ;) > > > > > --- > > > src/intel/vulkan/anv_device.c | 20 +++++++++++++++----- > > > 1 file changed, 15 insertions(+), 5 deletions(-) > > > > > > diff --git a/src/intel/vulkan/anv_device.c > > b/src/intel/vulkan/anv_device.c > > > index 38def35..5168331 100644 > > > --- a/src/intel/vulkan/anv_device.c > > > +++ b/src/intel/vulkan/anv_device.c > > > @@ -32,6 +32,7 @@ > > > #include "util/strtod.h" > > > #include "util/debug.h" > > > #include "util/build_id.h" > > > +#include "util/mesa-sha1.h" > > > #include "util/vk_util.h" > > > > > > #include "genxml/gen7_pack.h" > > > @@ -53,17 +54,26 @@ compiler_perf_log(void *data, const char *fmt, ...) > > > } > > > > > > static bool > > > -anv_device_get_cache_uuid(void *uuid) > > > +anv_device_get_cache_uuid(void *uuid, uint16_t pci_id) > > > { > > > const struct build_id_note *note = build_id_find_nhdr("libvulkan_ > > intel.so"); > > > if (!note) > > > return false; > > > > > > - unsigned len = build_id_length(note); > > > - if (len < VK_UUID_SIZE) > > > + unsigned build_id_len = build_id_length(note); > > > + if (build_id_len < VK_UUID_SIZE) > > > return false; > > > > Below, you're no longer copying the build_id into the pipelineCacheUUID. > > So checking `build_id_len >= VK_UUID_SIZE` is unneeded. > > > > Yes and no. I want to be sure that it's a SHA1 hash and not a timestamp. > Maybe we should check for >= 20 instead?
That sounds good. > > > - memcpy(uuid, build_id_data(note), VK_UUID_SIZE); > > > + uint8_t sha1[20]; > > > + struct mesa_sha1 *sha1_ctx = _mesa_sha1_init(); > > > + if (sha1_ctx == NULL) > > > + return false; > > > + > > > + _mesa_sha1_update(sha1_ctx, build_id_data(note), build_id_len); > > > + _mesa_sha1_update(sha1_ctx, &pci_id, sizeof(pci_id)); > > > + _mesa_sha1_final(sha1_ctx, sha1); > > > + > > > + memcpy(uuid, sha1, VK_UUID_SIZE); > > > > What really needs checking is `ARRAY_LEN(sha1) >= VK_UUID_SIZE`. That > > could be done as a static assert. > > > > sure. I was almost right. The check should be `VK_UUID_SIZE <= sizeof(sha1)`. memcpy cares about the size in bytes, not size in elems. > > > > > return true; > > > } > > > > > > @@ -148,7 +158,7 @@ anv_physical_device_init(struct anv_physical_device > > *device, > > > goto fail; > > > } > > > > > > - if (!anv_device_get_cache_uuid(device->uuid)) { > > > + if (!anv_device_get_cache_uuid(device->uuid, device->chipset_id)) { > > > result = vk_errorf(VK_ERROR_INITIALIZATION_FAILED, > > > "cannot generate UUID"); > > > goto fail; > > > -- > > > 2.5.0.400.gff86faf > > > > > > _______________________________________________ > > > mesa-dev mailing list > > > mesa-dev@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > > > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev