On Tue, Apr 13, 2021 at 12:13:30PM -0400, Chris von Recklinghausen wrote: > +static inline void get_e820_crc32(struct e820_table *table, void *buf) > { This should just return the CRC-32 value as a u32. There's no need for the 'void *buf' argument. Also like I said, compute_e820_crc32() would be a more logical name. > @@ -179,7 +133,8 @@ int arch_hibernation_header_save(void *addr, unsigned int max_size) > */ > rdr->cr3 = restore_cr3 & ~CR3_PCID_MASK; > > - return hibernation_e820_save(rdr->e820_digest); > + hibernation_e820_save(&rdr->e820_digest); > + return 0; Like I said, hibernation_e820_save() should just be inlined into here: rdr->e820_digest = compute_e820_crc32(e820_table_firmware) Having the helper function doesn't add anything. > /** > @@ -200,7 +155,7 @@ int arch_hibernation_header_restore(void *addr) > jump_address_phys = rdr->jump_address_phys; > restore_cr3 = rdr->cr3; > > - if (hibernation_e820_mismatch(rdr->e820_digest)) { > + if (hibernation_e820_mismatch(&rdr->e820_digest)) { Likewise, this should be just if (rdr->e820_digest != compute_e820_crc32(e820_table_firmware)) { - Eric