Loading...
iokit/Kernel/IONVRAMV3Handler.cpp xnu-12377.101.15 xnu-8792.61.2
--- xnu/xnu-12377.101.15/iokit/Kernel/IONVRAMV3Handler.cpp
+++ xnu/xnu-8792.61.2/iokit/Kernel/IONVRAMV3Handler.cpp
@@ -201,7 +201,8 @@
 
 	uint8_t                      *_nvramImage;
 
-	OSSharedPtr<OSDictionary>    _varDict;
+	OSSharedPtr<OSDictionary>    &_commonDict;
+	OSSharedPtr<OSDictionary>    &_systemDict;
 
 	uint32_t                     _commonSize;
 	uint32_t                     _systemSize;
@@ -212,53 +213,51 @@
 	uint32_t                     _currentOffset;
 
 	OSSharedPtr<OSArray>         _varEntries;
-
-	IORWLock                     *_variableLock;
-	IOLock                       *_controllerLock;
 
 	IOReturn unserializeImage(const uint8_t *image, IOByteCount length);
 	IOReturn reclaim(void);
 	uint32_t findCurrentBank(void);
-	size_t   getAppendSize(void);
 
 	static bool convertObjectToProp(uint8_t *buffer, uint32_t *length, const char *propSymbol, OSObject *propObject);
 	static bool convertPropToObject(const uint8_t *propName, uint32_t propNameLength, const uint8_t *propData, uint32_t propDataLength,
 	    OSSharedPtr<const OSSymbol>& propSymbol, OSSharedPtr<OSObject>& propObject);
 
 	IOReturn reloadInternal(void);
-	IOReturn setVariableInternal(const uuid_t varGuid, const char *variableName, OSObject *object);
 
 	void setEntryForRemove(struct nvram_v3_var_entry *v3Entry, bool system);
-	void findExistingEntry(const uuid_t varGuid, const char *varName, struct nvram_v3_var_entry **existing, unsigned int *existingIndex);
+	void findExistingEntry(const uuid_t *varGuid, const char *varName, struct nvram_v3_var_entry **existing, unsigned int *existingIndex);
 	IOReturn syncRaw(void);
 	IOReturn syncBlock(void);
+
 public:
 	virtual
 	~IONVRAMV3Handler() APPLE_KEXT_OVERRIDE;
-	IONVRAMV3Handler();
+	IONVRAMV3Handler(OSSharedPtr<OSDictionary> &commonDict, OSSharedPtr<OSDictionary> &systemDict);
+
 	static bool isValidImage(const uint8_t *image, IOByteCount length);
-	static  IONVRAMV3Handler *init(IODTNVRAM *provider, const uint8_t *image, IOByteCount length);
+
+	static  IONVRAMV3Handler *init(IODTNVRAM *provider, const uint8_t *image, IOByteCount length,
+	    OSSharedPtr<OSDictionary> &commonDict, OSSharedPtr<OSDictionary> &systemDict);
 
 	virtual bool     getNVRAMProperties(void) APPLE_KEXT_OVERRIDE;
-	virtual IOReturn unserializeVariables(void) APPLE_KEXT_OVERRIDE;
 	virtual IOReturn setVariable(const uuid_t varGuid, const char *variableName, OSObject *object) APPLE_KEXT_OVERRIDE;
 	virtual bool     setController(IONVRAMController *controller) APPLE_KEXT_OVERRIDE;
-	virtual IOReturn sync(void) APPLE_KEXT_OVERRIDE;
+	virtual bool     sync(void) APPLE_KEXT_OVERRIDE;
 	virtual IOReturn flush(const uuid_t guid, IONVRAMOperation op) APPLE_KEXT_OVERRIDE;
 	virtual void     reload(void) APPLE_KEXT_OVERRIDE;
 	virtual uint32_t getGeneration(void) const APPLE_KEXT_OVERRIDE;
 	virtual uint32_t getVersion(void) const APPLE_KEXT_OVERRIDE;
 	virtual uint32_t getSystemUsed(void) const APPLE_KEXT_OVERRIDE;
 	virtual uint32_t getCommonUsed(void) const APPLE_KEXT_OVERRIDE;
-	virtual bool     getSystemPartitionActive(void) const APPLE_KEXT_OVERRIDE;
-	virtual IOReturn getVarDict(OSSharedPtr<OSDictionary> &varDictCopy) APPLE_KEXT_OVERRIDE;
 };
 
 IONVRAMV3Handler::~IONVRAMV3Handler()
 {
 }
 
-IONVRAMV3Handler::IONVRAMV3Handler()
+IONVRAMV3Handler::IONVRAMV3Handler(OSSharedPtr<OSDictionary> &commonDict, OSSharedPtr<OSDictionary> &systemDict) :
+	_commonDict(commonDict),
+	_systemDict(systemDict)
 {
 }
 
@@ -275,21 +274,16 @@
 }
 
 IONVRAMV3Handler*
-IONVRAMV3Handler::init(IODTNVRAM *provider, const uint8_t *image, IOByteCount length)
+IONVRAMV3Handler::init(IODTNVRAM *provider, const uint8_t *image, IOByteCount length,
+    OSSharedPtr<OSDictionary> &commonDict, OSSharedPtr<OSDictionary> &systemDict)
 {
 	OSSharedPtr<IORegistryEntry> entry;
 	OSSharedPtr<OSObject>        prop;
 	bool                         propertiesOk;
 
-	IONVRAMV3Handler *handler = new IONVRAMV3Handler();
+	IONVRAMV3Handler *handler = new IONVRAMV3Handler(commonDict, systemDict);
 
 	handler->_provider = provider;
-
-	handler->_variableLock = IORWLockAlloc();
-	require(handler->_variableLock != nullptr, exit);
-
-	handler->_controllerLock = IOLockAlloc();
-	require(handler->_controllerLock != nullptr, exit);
 
 	propertiesOk = handler->getNVRAMProperties();
 	require_action(propertiesOk, exit, DEBUG_ERROR("Unable to get NVRAM properties\n"));
@@ -343,53 +337,89 @@
 IONVRAMV3Handler::flush(const uuid_t guid, IONVRAMOperation op)
 {
 	IOReturn ret = kIOReturnSuccess;
-	bool     flushSystem;
-	bool     flushCommon;
-
-	flushSystem = getSystemPartitionActive() && (uuid_compare(guid, gAppleSystemVariableGuid) == 0);
-	flushCommon = uuid_compare(guid, gAppleNVRAMGuid) == 0;
-
-	DEBUG_INFO("flushSystem=%d, flushCommon=%d\n", flushSystem, flushCommon);
-
-	NVRAMWRITELOCK(_variableLock);
-	if (flushSystem || flushCommon) {
-		const OSSymbol                    *canonicalKey;
-		OSSharedPtr<OSDictionary>         dictCopy;
+
+	if ((_systemDict != nullptr) && (uuid_compare(guid, gAppleSystemVariableGuid) == 0)) {
+		// System dictionary contains keys that are only using the system GUID
+		const OSSymbol                    *key;
+		OSSharedPtr<OSDictionary>         systemCopy;
 		OSSharedPtr<OSCollectionIterator> iter;
 		uuid_string_t                     uuidString;
 
-		dictCopy = OSDictionary::withDictionary(_varDict.get());
-		iter = OSCollectionIterator::withCollection(dictCopy.get());
-		require_action(dictCopy && iter, exit, ret = kIOReturnNoMemory);
-
-		while ((canonicalKey = OSDynamicCast(OSSymbol, iter->getNextObject()))) {
-			const char *varName;
-			uuid_t     varGuid;
-			bool       clear;
-
-			parseVariableName(canonicalKey->getCStringNoCopy(), &varGuid, &varName);
-
-			uuid_unparse(varGuid, uuidString);
-
-			clear = ((flushSystem && (uuid_compare(varGuid, gAppleSystemVariableGuid) == 0)) ||
-			    (flushCommon && (uuid_compare(varGuid, gAppleSystemVariableGuid) != 0))) &&
-			    verifyPermission(op, varGuid, varName, getSystemPartitionActive(), true);
-
-			if (clear) {
-				DEBUG_INFO("Clearing entry for %s:%s\n", uuidString, varName);
-				setVariableInternal(varGuid, varName, nullptr);
+		systemCopy = OSDictionary::withDictionary(_systemDict.get());
+		iter = OSCollectionIterator::withCollection(systemCopy.get());
+		if ((systemCopy == nullptr) || (iter == nullptr)) {
+			ret = kIOReturnNoMemory;
+			goto exit;
+		}
+
+		DEBUG_INFO("Flushing system region...\n");
+
+		uuid_unparse(gAppleSystemVariableGuid, uuidString);
+		while ((key = OSDynamicCast(OSSymbol, iter->getNextObject()))) {
+			if (verifyPermission(op, gAppleSystemVariableGuid, key)) {
+				DEBUG_INFO("Clearing entry for %s:%s\n", uuidString, key->getCStringNoCopy());
+				// Using setVariable() instead of setEntryForRemove() to handle any GUID logic
+				// for system region
+				setVariable(guid, key->getCStringNoCopy(), nullptr);
 			} else {
-				DEBUG_INFO("Keeping entry for %s:%s\n", uuidString, varName);
-			}
-		}
-
-		_newData = true;
+				DEBUG_INFO("Keeping entry for %s:%s\n", uuidString, key->getCStringNoCopy());
+			}
+		}
+
+		DEBUG_INFO("system dictionary flushed\n");
+	} else if ((_commonDict != nullptr) && (uuid_compare(guid, gAppleNVRAMGuid) == 0)) {
+		// Common dictionary contains everything that is not system this goes through our entire
+		// store and clears anything that is permitted
+		struct nvram_v3_var_entry *v3Entry = nullptr;
+		OSData                    *entryContainer = nullptr;
+		OSSharedPtr<OSDictionary> newCommonDict;
+		uuid_string_t             uuidString;
+
+		DEBUG_INFO("Flushing common region...\n");
+
+		newCommonDict = OSDictionary::withCapacity(_commonDict->getCapacity());
+
+		if (newCommonDict == nullptr) {
+			ret = kIOReturnNoMemory;
+			goto exit;
+		}
+
+		for (unsigned int index = 0; index < _varEntries->getCount(); index++) {
+			entryContainer = (OSDynamicCast(OSData, _varEntries->getObject(index)));
+			v3Entry = (struct nvram_v3_var_entry *)entryContainer->getBytesNoCopy();
+			const char *entryName = (const char *)v3Entry->header.name_data_buf;
+
+			// Skip system variables if there is a system region
+			if ((_systemSize != 0) && uuid_compare(v3Entry->header.guid, gAppleSystemVariableGuid) == 0) {
+				continue;
+			}
+
+			uuid_unparse(v3Entry->header.guid, uuidString);
+			if (verifyPermission(op, v3Entry->header.guid, entryName)) {
+				DEBUG_INFO("Clearing entry for %s:%s\n", uuidString, entryName);
+				setEntryForRemove(v3Entry, false);
+			} else {
+				DEBUG_INFO("Keeping entry for %s:%s\n", uuidString, entryName);
+				newCommonDict->setObject(entryName, _commonDict->getObject(entryName));
+			}
+		}
+
+		_commonDict = newCommonDict;
+	}
+
+	_newData = true;
+
+	if (_provider->_diags) {
+		OSSharedPtr<OSNumber> val = OSNumber::withNumber(getSystemUsed(), 32);
+		_provider->_diags->setProperty(kNVRAMSystemUsedKey, val.get());
+
+		val = OSNumber::withNumber(getCommonUsed(), 32);
+		_provider->_diags->setProperty(kNVRAMCommonUsedKey, val.get());
 	}
 
 	DEBUG_INFO("_commonUsed %#x, _systemUsed %#x\n", _commonUsed, _systemUsed);
 
 exit:
-	NVRAMRWUNLOCK(_variableLock);
 	return ret;
 }
 
@@ -404,17 +434,15 @@
 	const struct v3_var_header   *storeVar;
 	OSData                       *entryContainer;
 
-	NVRAMLOCKASSERTHELD(_controllerLock);
-
 	controllerBank = findCurrentBank();
 
 	if (_currentBank != controllerBank) {
-		DEBUG_ERROR("_currentBank %#x != controllerBank %#x\n", _currentBank, controllerBank);
+		DEBUG_ERROR("_currentBank %#x != controllerBank %#x", _currentBank, controllerBank);
 	}
 
 	_currentBank = controllerBank;
 
-	controllerImage = (uint8_t *)IOMallocZeroData(_bankSize);
+	controllerImage = (uint8_t *)IOMallocData(_bankSize);
 
 	_nvramController->select(_currentBank);
 	_nvramController->read(0, controllerImage, _bankSize);
@@ -433,7 +461,6 @@
 	// as VAR_NEW_STATE_NONE meaning no action needed
 	// Otherwise if the data is different or it is not found on the controller image we mark it as VAR_NEW_STATE_APPEND
 	// which will have us invalidate the existing entry if there is one and append it on the next save
-	NVRAMREADLOCK(_variableLock);
 	for (unsigned int i = 0; i < _varEntries->getCount(); i++) {
 		uint32_t offset = sizeof(struct v3_store_header);
 		uint32_t latestOffset;
@@ -452,7 +479,7 @@
 				uint8_t state = prevVarHeader->state & VAR_DELETED & VAR_IN_DELETED_TRANSITION;
 
 				ret = _nvramController->write(prevOffset + offsetof(struct v3_var_header, state), &state, sizeof(state));
-				require_noerr_action(ret, unlock, DEBUG_ERROR("existing state w fail, ret=%#x\n", ret));
+				require_noerr_action(ret, exit, DEBUG_ERROR("existing state w fail, ret=%#x\n", ret));
 			}
 
 			prevOffset = latestOffset;
@@ -490,12 +517,15 @@
 			}
 		}
 	}
+
 	ret = find_current_offset_in_image(controllerImage, _bankSize, &_currentOffset);
-	require_noerr_action(ret, unlock, DEBUG_ERROR("Unidentified bytes in image\n"));
+	if (ret != kIOReturnSuccess) {
+		DEBUG_ERROR("Unidentified bytes in image, reclaiming\n");
+		ret = reclaim();
+		require_noerr_action(ret, exit, DEBUG_ERROR("Reclaim byte recovery failed, invalid controller state!!! ret=%#x\n", ret));
+	}
 	DEBUG_INFO("New _currentOffset=%#x\n", _currentOffset);
 
-unlock:
-	NVRAMRWUNLOCK(_variableLock);
 exit:
 	IOFreeData(controllerImage, _bankSize);
 	return ret;
@@ -512,18 +542,13 @@
 void
 IONVRAMV3Handler::setEntryForRemove(struct nvram_v3_var_entry *v3Entry, bool system)
 {
-	OSSharedPtr<const OSSymbol> canonicalKey;
-	const char                  *variableName;
-	uint32_t                    variableSize;
-
-	// Anyone calling setEntryForRemove should've already held the lock for write.
-	NVRAMRWLOCKASSERTEXCLUSIVE(_variableLock);
+	const char * variableName;
+	uint32_t variableSize;
 
 	require_action(v3Entry != nullptr, exit, DEBUG_INFO("remove with no entry\n"));
 
 	variableName = (const char *)v3Entry->header.name_data_buf;
 	variableSize = (uint32_t)variable_length(&v3Entry->header);
-	canonicalKey = keyWithGuidAndCString(v3Entry->header.guid, variableName);
 
 	if (v3Entry->new_state == VAR_NEW_STATE_REMOVE) {
 		DEBUG_INFO("entry %s already marked for remove\n", variableName);
@@ -532,14 +557,17 @@
 
 		v3Entry->new_state = VAR_NEW_STATE_REMOVE;
 
-		_varDict->removeObject(canonicalKey.get());
-
 		if (system) {
+			_provider->_systemDict->removeObject(variableName);
+
 			if (_systemUsed < variableSize) {
 				panic("Invalid _systemUsed size\n");
 			}
+
 			_systemUsed -= variableSize;
 		} else {
+			_provider->_commonDict->removeObject(variableName);
+
 			if (_commonUsed < variableSize) {
 				panic("Invalid _commonUsed size\n");
 			}
@@ -549,7 +577,8 @@
 		if (_provider->_diags) {
 			_provider->_diags->logVariable(getPartitionTypeForGUID(v3Entry->header.guid),
 			    kIONVRAMOperationDelete,
-			    variableName);
+			    variableName,
+			    nullptr);
 		}
 	}
 
@@ -558,7 +587,7 @@
 }
 
 void
-IONVRAMV3Handler::findExistingEntry(const uuid_t varGuid, const char *varName, struct nvram_v3_var_entry **existing, unsigned int *existingIndex)
+IONVRAMV3Handler::findExistingEntry(const uuid_t *varGuid, const char *varName, struct nvram_v3_var_entry **existing, unsigned int *existingIndex)
 {
 	struct nvram_v3_var_entry *v3Entry = nullptr;
 	OSData                    *entryContainer = nullptr;
@@ -572,9 +601,9 @@
 		if ((v3Entry->header.nameSize == nameLen) &&
 		    (memcmp(v3Entry->header.name_data_buf, varName, nameLen) == 0)) {
 			if (varGuid) {
-				if (uuid_compare(varGuid, v3Entry->header.guid) == 0) {
+				if (uuid_compare(*varGuid, v3Entry->header.guid) == 0) {
 					uuid_string_t uuidString;
-					uuid_unparse(varGuid, uuidString);
+					uuid_unparse(*varGuid, uuidString);
 					DEBUG_INFO("found existing entry for %s:%s, e_off=%#lx, len=%#lx, new_state=%#x\n", uuidString, varName,
 					    v3Entry->existing_offset, variable_length(&v3Entry->header), v3Entry->new_state);
 					break;
@@ -602,67 +631,61 @@
 IOReturn
 IONVRAMV3Handler::unserializeImage(const uint8_t *image, IOByteCount length)
 {
-	IOReturn                     ret = kIOReturnInvalid;
-	const struct v3_store_header *storeHeader;
-
-	require(isValidImage(image, length), exit);
-
-	storeHeader = (const struct v3_store_header *)image;
-	require_action(storeHeader->size == (uint32_t)length, exit,
-	    DEBUG_ERROR("Image size %#x != header size %#x\n", (unsigned int)length, storeHeader->size));
-
-	_generation = storeHeader->generation;
-	_systemSize = storeHeader->system_size;
-	_commonSize = storeHeader->common_size - sizeof(struct v3_store_header);
-
-	_systemUsed = 0;
-	_commonUsed = 0;
-
-	if (_nvramImage) {
-		IOFreeData(_nvramImage, _bankSize);
-	}
-
-	_varEntries.reset();
-	_varEntries = OSArray::withCapacity(40);
-
-	_nvramImage = IONewData(uint8_t, length);
-	_bankSize = (uint32_t)length;
-	bcopy(image, _nvramImage, _bankSize);
-
-	ret = kIOReturnSuccess;
-
-exit:
-	return ret;
-}
-
-IOReturn
-IONVRAMV3Handler::unserializeVariables(void)
-{
-	IOReturn                     ret = kIOReturnSuccess;
 	OSSharedPtr<const OSSymbol>  propSymbol;
 	OSSharedPtr<OSObject>        propObject;
 	OSSharedPtr<OSData>          entryContainer;
+	const struct v3_store_header *storeHeader;
+	IOReturn                     ret = kIOReturnSuccess;
+	size_t                       existingSize;
 	struct nvram_v3_var_entry    *v3Entry;
 	const struct v3_var_header   *header;
 	size_t                       offset = sizeof(struct v3_store_header);
 	uint32_t                     crc;
 	unsigned int                 i;
 	bool                         system;
+	OSDictionary                 *dict;
 	uuid_string_t                uuidString;
-	size_t                       existingSize;
-
-	if (_systemSize || _commonSize) {
-		_varDict = OSDictionary::withCapacity(1);
-	}
-
-	while ((offset + sizeof(struct v3_var_header)) < _bankSize) {
+
+	require(isValidImage(image, length), exit);
+
+	storeHeader = (const struct v3_store_header *)image;
+	require_action(storeHeader->size == (uint32_t)length, exit,
+	    DEBUG_ERROR("Image size %#x != header size %#x\n", (unsigned int)length, storeHeader->size));
+
+	_generation = storeHeader->generation;
+	_systemSize = storeHeader->system_size;
+	_commonSize = storeHeader->common_size - sizeof(struct v3_store_header);
+
+	_systemUsed = 0;
+	_commonUsed = 0;
+
+	if (_nvramImage) {
+		IOFreeData(_nvramImage, _bankSize);
+	}
+
+	_varEntries.reset();
+	_varEntries = OSArray::withCapacity(40);
+
+	_nvramImage = IONewData(uint8_t, length);
+	_bankSize = (uint32_t)length;
+	bcopy(image, _nvramImage, _bankSize);
+
+	if (_systemSize) {
+		_systemDict = OSDictionary::withCapacity(1);
+	}
+
+	if (_commonSize) {
+		_commonDict = OSDictionary::withCapacity(1);
+	}
+
+	while ((offset + sizeof(struct v3_var_header)) < length) {
 		struct nvram_v3_var_entry *existingEntry = nullptr;
 		unsigned int              existingIndex = 0;
 
-		header = (const struct v3_var_header *)(_nvramImage + offset);
+		header = (const struct v3_var_header *)(image + offset);
 
 		for (i = 0; i < sizeof(struct v3_var_header); i++) {
-			if ((_nvramImage[offset + i] != 0) && (_nvramImage[offset + i] != 0xFF)) {
+			if ((image[offset + i] != 0) && (image[offset + i] != 0xFF)) {
 				break;
 			}
 		}
@@ -672,7 +695,7 @@
 			break;
 		}
 
-		if (!valid_variable_header(header, _bankSize - offset)) {
+		if (!valid_variable_header(header, length - offset)) {
 			DEBUG_ERROR("invalid header @ %#lx\n", offset);
 			offset += sizeof(struct v3_var_header);
 			continue;
@@ -704,7 +727,7 @@
 		v3Entry->new_state = VAR_NEW_STATE_NONE;
 
 		// safe guard for any strange duplicate entries in the store
-		findExistingEntry(v3Entry->header.guid, (const char *)v3Entry->header.name_data_buf, &existingEntry, &existingIndex);
+		findExistingEntry(&v3Entry->header.guid, (const char *)v3Entry->header.name_data_buf, &existingEntry, &existingIndex);
 
 		if (existingEntry != nullptr) {
 			existingSize = variable_length(&existingEntry->header);
@@ -722,26 +745,25 @@
 
 		system = (_systemSize != 0) && (uuid_compare(v3Entry->header.guid, gAppleSystemVariableGuid) == 0);
 		if (system) {
+			dict = _systemDict.get();
 			_systemUsed = _systemUsed + (uint32_t)variable_length(header) - (uint32_t)existingSize;
 		} else {
+			dict = _commonDict.get();
 			_commonUsed = _commonUsed + (uint32_t)variable_length(header) - (uint32_t)existingSize;
 		}
 
 		if (convertPropToObject(v3Entry->header.name_data_buf, v3Entry->header.nameSize,
 		    v3Entry->header.name_data_buf + v3Entry->header.nameSize, v3Entry->header.dataSize,
 		    propSymbol, propObject)) {
-			OSSharedPtr<const OSSymbol> canonicalKey = keyWithGuidAndCString(v3Entry->header.guid, (const char *)v3Entry->header.name_data_buf);
-
-			DEBUG_INFO("adding %s, variableLength=%zu, dataLength=%u, system=%d\n",
-			    canonicalKey->getCStringNoCopy(), variable_length(header), v3Entry->header.dataSize, system);
-
-			_varDict->setObject(canonicalKey.get(), propObject.get());
+			DEBUG_INFO("adding %s, dataLength=%u, system=%d\n",
+			    propSymbol->getCStringNoCopy(), v3Entry->header.dataSize, system);
+
+			dict->setObject(propSymbol.get(), propObject.get());
 
 			if (_provider->_diags) {
-				_provider->_diags->logVariable(getPartitionTypeForGUID(v3Entry->header.guid),
+				_provider->_diags->logVariable(_provider->getDictionaryType(dict),
 				    kIONVRAMOperationInit, propSymbol.get()->getCStringNoCopy(),
-				    (void *)(uintptr_t)v3Entry->header.dataSize,
-				    (void *)(uintptr_t)offset);
+				    (void *)(uintptr_t)(header->name_data_buf + header->nameSize));
 			}
 		}
 		IOFreeData(v3Entry, nvram_v3_var_container_size(header));
@@ -752,12 +774,9 @@
 	_currentOffset = (uint32_t)offset;
 
 	DEBUG_ALWAYS("_commonSize %#x, _systemSize %#x, _currentOffset %#x\n", _commonSize, _systemSize, _currentOffset);
-
-	ret = handleEphDM();
-	verify_noerr_action(ret, panic("handleEphDM failed with ret=%08x", ret));
-
 	DEBUG_INFO("_commonUsed %#x, _systemUsed %#x\n", _commonUsed, _systemUsed);
 
+exit:
 	_newData = true;
 
 	if (_provider->_diags) {
@@ -774,33 +793,48 @@
 }
 
 IOReturn
-IONVRAMV3Handler::setVariableInternal(const uuid_t varGuid, const char *variableName, OSObject *object)
-{
-	struct nvram_v3_var_entry   *v3Entry = nullptr;
-	struct nvram_v3_var_entry   *newV3Entry;
-	OSSharedPtr<OSData>         newContainer;
-	OSSharedPtr<const OSSymbol> canonicalKey;
-	bool                        unset = (object == nullptr);
-	bool                        system = false;
-	IOReturn                    ret = kIOReturnSuccess;
-	size_t                      entryNameLen = strlen(variableName) + 1;
-	unsigned int                existingEntryIndex;
-	uint32_t                    dataSize = 0;
-	size_t                      existingVariableSize = 0;
-	size_t                      newVariableSize = 0;
-	size_t                      newEntrySize;
-	uuid_string_t               uuidString;
-
-	// Anyone calling setVariableInternal should've already held the lock for write.
-	NVRAMRWLOCKASSERTEXCLUSIVE(_variableLock);
-
-	system = (uuid_compare(varGuid, gAppleSystemVariableGuid) == 0);
-	canonicalKey = keyWithGuidAndCString(varGuid, variableName);
+IONVRAMV3Handler::setVariable(const uuid_t varGuid, const char *variableName, OSObject *object)
+{
+	struct nvram_v3_var_entry *v3Entry = nullptr;
+	struct nvram_v3_var_entry *newV3Entry;
+	OSSharedPtr<OSData>       newContainer;
+	bool                      unset = (object == nullptr);
+	bool                      system = false;
+	IOReturn                  ret = kIOReturnSuccess;
+	size_t                    entryNameLen = strlen(variableName) + 1;
+	unsigned int              existingEntryIndex;
+	uint32_t                  dataSize = 0;
+	size_t                    existingVariableSize = 0;
+	size_t                    newVariableSize = 0;
+	size_t                    newEntrySize;
+	uuid_t                    destGuid;
+	uuid_string_t             uuidString;
+
+	if (_systemSize != 0) {
+		// System region case, if they're using the GUID directly or it's on the system allow list
+		// force it to use the System GUID
+		if ((uuid_compare(varGuid, gAppleSystemVariableGuid) == 0) || variableInAllowList(variableName)) {
+			system = true;
+			uuid_copy(destGuid, gAppleSystemVariableGuid);
+		} else {
+			uuid_copy(destGuid, varGuid);
+		}
+	} else {
+		// No system region, store System GUID as Common GUID
+		if ((uuid_compare(varGuid, gAppleSystemVariableGuid) == 0) || variableInAllowList(variableName)) {
+			uuid_copy(destGuid, gAppleNVRAMGuid);
+		} else {
+			uuid_copy(destGuid, varGuid);
+		}
+	}
 
 	uuid_unparse(varGuid, uuidString);
 	DEBUG_INFO("setting %s:%s, system=%d, current var count=%u\n", uuidString, variableName, system, _varEntries->getCount());
 
-	findExistingEntry(varGuid, variableName, &v3Entry, &existingEntryIndex);
+	uuid_unparse(destGuid, uuidString);
+	DEBUG_INFO("using %s, _commonUsed %#x, _systemUsed %#x\n", uuidString, _commonUsed, _systemUsed);
+
+	findExistingEntry(&destGuid, variableName, &v3Entry, &existingEntryIndex);
 
 	if (unset == true) {
 		setEntryForRemove(v3Entry, system);
@@ -837,7 +871,7 @@
 		newV3Entry->header.nameSize = (uint32_t)entryNameLen;
 		newV3Entry->header.dataSize = dataSize;
 		newV3Entry->header.crc = crc32(0, newV3Entry->header.name_data_buf + entryNameLen, dataSize);
-		memcpy(newV3Entry->header.guid, varGuid, sizeof(gAppleNVRAMGuid));
+		memcpy(newV3Entry->header.guid, &destGuid, sizeof(gAppleNVRAMGuid));
 		newV3Entry->new_state = VAR_NEW_STATE_APPEND;
 
 		if (v3Entry) {
@@ -854,16 +888,14 @@
 
 		if (system) {
 			_systemUsed = _systemUsed + (uint32_t)newVariableSize - (uint32_t)existingVariableSize;
+			_provider->_systemDict->setObject(variableName, object);
 		} else {
 			_commonUsed = _commonUsed + (uint32_t)newVariableSize - (uint32_t)existingVariableSize;
-		}
-
-		_varDict->setObject(canonicalKey.get(), object);
+			_provider->_commonDict->setObject(variableName, object);
+		}
 
 		if (_provider->_diags) {
-			_provider->_diags->logVariable(getPartitionTypeForGUID(varGuid),
-			    kIONVRAMOperationWrite, variableName,
-			    (void *)(uintptr_t)dataSize);
+			_provider->_diags->logVariable(getPartitionTypeForGUID(destGuid), kIONVRAMOperationWrite, variableName, (void *)(uintptr_t)dataSize);
 		}
 
 		IOFreeData(newV3Entry, newEntrySize);
@@ -885,43 +917,6 @@
 	return ret;
 }
 
-IOReturn
-IONVRAMV3Handler::setVariable(const uuid_t varGuid, const char *variableName, OSObject *object)
-{
-	uuid_t destGuid;
-	IOReturn ret = kIOReturnError;
-
-	if (strcmp(variableName, "reclaim-int") == 0) {
-		NVRAMLOCK(_controllerLock);
-		ret = reclaim();
-		NVRAMUNLOCK(_controllerLock);
-		return ret;
-	}
-
-	if (getSystemPartitionActive()) {
-		// System region case, if they're using the GUID directly or it's on the system allow list
-		// force it to use the System GUID
-		if ((uuid_compare(varGuid, gAppleSystemVariableGuid) == 0) || variableInAllowList(variableName)) {
-			uuid_copy(destGuid, gAppleSystemVariableGuid);
-		} else {
-			uuid_copy(destGuid, varGuid);
-		}
-	} else {
-		// No system region, store System GUID as Common GUID
-		if ((uuid_compare(varGuid, gAppleSystemVariableGuid) == 0) || variableInAllowList(variableName)) {
-			uuid_copy(destGuid, gAppleNVRAMGuid);
-		} else {
-			uuid_copy(destGuid, varGuid);
-		}
-	}
-
-	NVRAMWRITELOCK(_variableLock);
-	ret = setVariableInternal(destGuid, variableName, object);
-	NVRAMRWUNLOCK(_variableLock);
-
-	return ret;
-}
-
 uint32_t
 IONVRAMV3Handler::findCurrentBank(void)
 {
@@ -929,8 +924,6 @@
 	uint32_t               maxGen = 0;
 	uint32_t               currentBank = 0;
 
-	NVRAMLOCKASSERTHELD(_controllerLock);
-
 	for (unsigned int i = 0; i < _bankCount; i++) {
 		_nvramController->select(i);
 		_nvramController->read(0, (uint8_t *)&storeHeader, sizeof(storeHeader));
@@ -941,7 +934,7 @@
 		}
 	}
 
-	DEBUG_ALWAYS("currentBank=%#x, gen=%#x\n", currentBank, maxGen);
+	DEBUG_ALWAYS("currentBank=%#x, gen=%#x", currentBank, maxGen);
 
 	return currentBank;
 }
@@ -950,8 +943,6 @@
 IONVRAMV3Handler::setController(IONVRAMController *controller)
 {
 	IOReturn ret = kIOReturnSuccess;
-
-	NVRAMLOCK(_controllerLock);
 
 	if (_nvramController == NULL) {
 		_nvramController = controller;
@@ -964,39 +955,34 @@
 	if (_resetData) {
 		_resetData = false;
 		DEBUG_ERROR("_resetData set, issuing reclaim recovery\n");
-		goto reclaim;
-	}
-
-	if (reloadInternal() == kIOReturnSuccess) {
+		ret = reclaim();
+		require_noerr_action(ret, exit, DEBUG_ERROR("Reclaim recovery failed, invalid controller state!!! ret=%#x\n", ret));
 		goto exit;
 	}
 
-reclaim:
-	ret = reclaim();
-	require_noerr_action(ret, exit, DEBUG_ERROR("Reclaim recovery failed, invalid controller state!!! ret=%#x\n", ret));
+	ret = reloadInternal();
+	if (ret != kIOReturnSuccess) {
+		DEBUG_ERROR("Invalid image found, issuing reclaim recovery\n");
+		ret = reclaim();
+		require_noerr_action(ret, exit, DEBUG_ERROR("Reclaim recovery failed, invalid controller state!!! ret=%#x\n", ret));
+	}
+
 exit:
-	NVRAMUNLOCK(_controllerLock);
 	return ret == kIOReturnSuccess;
 }
 
 IOReturn
 IONVRAMV3Handler::reclaim(void)
 {
-	IOReturn             ret;
-	struct               v3_store_header newStoreHeader;
-	struct               v3_var_header *varHeader;
-	struct               nvram_v3_var_entry *varEntry;
-	OSData               *entryContainer;
-	size_t               new_bank_offset = sizeof(struct v3_store_header);
-	uint32_t             next_bank = (_currentBank + 1) % _bankCount;
-	uint8_t              *bankData;
-	OSSharedPtr<OSArray> remainingEntries;
+	IOReturn ret;
+	struct   v3_store_header newStoreHeader;
+	struct   v3_var_header *varHeader;
+	struct   nvram_v3_var_entry *varEntry;
+	OSData   *entryContainer;
+	size_t   new_bank_offset = sizeof(struct v3_store_header);
+	uint32_t next_bank = (_currentBank + 1) % _bankCount;
 
 	DEBUG_INFO("called\n");
-	NVRAMLOCKASSERTHELD(_controllerLock);
-
-	bankData = (uint8_t *)IOMallocZeroData(_bankSize);
-	require_action(bankData != nullptr, exit, ret = kIOReturnNoMemory);
 
 	ret = _nvramController->select(next_bank);
 	verify_noerr_action(ret, DEBUG_INFO("select of bank %#08x failed\n", next_bank));
@@ -1005,10 +991,6 @@
 	verify_noerr_action(ret, DEBUG_INFO("eraseBank failed, ret=%#08x\n", ret));
 
 	_currentBank = next_bank;
-
-	NVRAMREADLOCK(_variableLock);
-
-	remainingEntries = OSArray::withCapacity(_varEntries->getCapacity());
 
 	for (unsigned int i = 0; i < _varEntries->getCount(); i++) {
 		entryContainer = OSDynamicCast(OSData, _varEntries->getObject(i));
@@ -1018,19 +1000,16 @@
 		DEBUG_INFO("entry %u %s, new_state=%#x, e_offset=%#lx, state=%#x\n",
 		    i, varEntry->header.name_data_buf, varEntry->new_state, varEntry->existing_offset, varHeader->state);
 
-		if ((varEntry->new_state == VAR_NEW_STATE_NONE) ||
-		    (varEntry->new_state == VAR_NEW_STATE_APPEND)) {
-			varHeader->state = VAR_ADDED;
-
-			memcpy(bankData + new_bank_offset, (uint8_t *)varHeader, variable_length(varHeader));
-
-			varEntry->new_state = VAR_NEW_STATE_NONE;
+		if (varEntry->new_state == VAR_NEW_STATE_NONE) {
+			ret = _nvramController->write(new_bank_offset, (uint8_t *)varHeader, variable_length(varHeader));
+			require_noerr_action(ret, exit, DEBUG_ERROR("var write failed, ret=%08x\n", ret));
+
 			varEntry->existing_offset = new_bank_offset;
 			new_bank_offset += variable_length(varHeader);
-
-			remainingEntries->setObject(entryContainer);
 		} else {
-			// entryContainer not added to remainingEntries, entry dropped
+			// Set existing offset to 0 so that they will either be appended
+			// or any remaining removals will be dropped
+			varEntry->existing_offset = 0;
 		}
 	}
 
@@ -1040,162 +1019,130 @@
 
 	newStoreHeader.generation = _generation;
 
-	memcpy(bankData, (uint8_t *)&newStoreHeader, sizeof(newStoreHeader));
-
-	ret = _nvramController->write(0, bankData, new_bank_offset);
-	require_noerr_action(ret, unlock, DEBUG_ERROR("reclaim bank write failed, ret=%08x\n", ret));
+	ret = _nvramController->write(0, (uint8_t *)&newStoreHeader, sizeof(newStoreHeader));
+	require_noerr_action(ret, exit, DEBUG_ERROR("store header write failed, ret=%08x\n", ret));
 
 	_currentOffset = (uint32_t)new_bank_offset;
 
-	DEBUG_INFO("Reclaim complete, _currentBank=%u _generation=%u, _currentOffset=%#x\n", _currentBank, _generation, _currentOffset);
-
-	_newData = false;
-	_varEntries.reset(remainingEntries.get(), OSRetain);
-
-unlock:
-	NVRAMRWUNLOCK(_variableLock);
+	DEBUG_INFO("Reclaim complete, _generation=%u, _currentOffset=%#x\n", _generation, _currentOffset);
+
 exit:
-	IOFreeData(bankData, _bankSize);
-
 	return ret;
 }
 
-size_t
-IONVRAMV3Handler::getAppendSize(void)
-{
-	struct nvram_v3_var_entry *varEntry;
-	struct v3_var_header      *varHeader;
-	OSData                    *entryContainer;
-	size_t                    appendSize = 0;
-
-	NVRAMRWLOCKASSERTHELD(_variableLock);
+IOReturn
+IONVRAMV3Handler::syncRaw(void)
+{
+	IOReturn             ret = kIOReturnSuccess;
+	size_t               varEndOffset;
+	size_t               varStartOffset;
+	struct               nvram_v3_var_entry *varEntry;
+	struct               v3_var_header *varHeader;
+	OSData               *entryContainer;
+	OSSharedPtr<OSArray> remainingEntries;
+
+	require_action(_nvramController != nullptr, exit, DEBUG_INFO("No _nvramController\n"));
+	require_action(_newData == true, exit, DEBUG_INFO("No _newData to sync\n"));
+	require_action(_bankSize != 0, exit, DEBUG_INFO("No nvram size info\n"));
+
+	DEBUG_INFO("_varEntries->getCount()=%#x\n", _varEntries->getCount());
+
+	remainingEntries = OSArray::withCapacity(_varEntries->getCapacity());
 
 	for (unsigned int i = 0; i < _varEntries->getCount(); i++) {
+		size_t space_needed = 0;
+		uint8_t state;
+
 		entryContainer = OSDynamicCast(OSData, _varEntries->getObject(i));
 		varEntry = (struct nvram_v3_var_entry *)entryContainer->getBytesNoCopy();
 		varHeader = &varEntry->header;
 
+		DEBUG_INFO("%s new_state=%d, e_off=%#lx, c_off=%#x, uuid=%x%x, nameSize=%#x, dataSize=%#x\n",
+		    varEntry->header.name_data_buf,
+		    varEntry->new_state, varEntry->existing_offset, _currentOffset,
+		    varHeader->guid[0], varHeader->guid[1],
+		    varHeader->nameSize, varHeader->dataSize);
+
 		if (varEntry->new_state == VAR_NEW_STATE_APPEND) {
-			appendSize += variable_length(varHeader);
-		}
-	}
-
-	return appendSize;
-}
-
-IOReturn
-IONVRAMV3Handler::syncRaw(void)
-{
-	IOReturn                  ret = kIOReturnSuccess;
-	struct nvram_v3_var_entry *varEntry;
-	struct v3_var_header      *varHeader;
-	OSData                    *entryContainer;
-	OSSharedPtr<OSArray>      remainingEntries;
-	uint8_t                   *appendBuffer = nullptr;
-	size_t                    appendBufferOffset = 0;
-	size_t                    *invalidateOffsets = nullptr;
-	size_t                    invalidateOffsetsCount = 0;
-	size_t                    invalidateOffsetIndex = 0;
-
-	require_action(_nvramController != nullptr, exit, DEBUG_INFO("No _nvramController\n"));
-	require_action(_newData == true, exit, DEBUG_INFO("No _newData to sync\n"));
-	require_action(_bankSize != 0, exit, DEBUG_INFO("No nvram size info\n"));
-
-	NVRAMREADLOCK(_variableLock);
-	DEBUG_INFO("_varEntries->getCount()=%#x\n", _varEntries->getCount());
-
-	if (getAppendSize() + _currentOffset < _bankSize) {
-		// No reclaim, build append and invalidate list
-		remainingEntries = OSArray::withCapacity(_varEntries->getCapacity());
-
-		appendBuffer = (uint8_t *)IOMallocZeroData(_bankSize);
-		require_action(appendBuffer, unlock, ret = kIOReturnNoMemory);
-
-		invalidateOffsetsCount = _varEntries->getCount();
-		invalidateOffsets = (size_t *)IOMallocZeroData(invalidateOffsetsCount * sizeof(size_t));
-		require_action(invalidateOffsets, unlock, ret = kIOReturnNoMemory);
-
-		for (unsigned int i = 0; i < _varEntries->getCount(); i++) {
-			entryContainer = OSDynamicCast(OSData, _varEntries->getObject(i));
-			varEntry = (struct nvram_v3_var_entry *)entryContainer->getBytesNoCopy();
-			varHeader = &varEntry->header;
-
-			DEBUG_INFO("entry %s, new_state=%#02x state=%#02x, existing_offset=%#zx\n",
-			    varEntry->header.name_data_buf, varEntry->new_state, varEntry->header.state, varEntry->existing_offset);
-
-			if (varEntry->new_state == VAR_NEW_STATE_APPEND) {
-				size_t varSize = variable_length(varHeader);
-				size_t prevOffset = varEntry->existing_offset;
-
-				varHeader->state = VAR_ADDED;
-				varEntry->existing_offset = _currentOffset + appendBufferOffset;
-				varEntry->new_state = VAR_NEW_STATE_NONE;
-
-				DEBUG_INFO("Appending %s in append buffer offset %#zx, actual offset %#zx, prevOffset %#zx, varsize=%#zx\n",
-				    varEntry->header.name_data_buf, appendBufferOffset, varEntry->existing_offset, prevOffset, varSize);
-
-				// Write to append buffer
-				memcpy(appendBuffer + appendBufferOffset, (uint8_t *)varHeader, varSize);
-				appendBufferOffset += varSize;
-
-				if (prevOffset) {
-					invalidateOffsets[invalidateOffsetIndex++] = prevOffset;
+			space_needed = variable_length(varHeader);
+
+			// reclaim if needed
+			if ((_currentOffset + space_needed) > _bankSize) {
+				ret = reclaim();
+				require_noerr_action(ret, exit, DEBUG_ERROR("reclaim fail, ret=%#x\n", ret));
+
+				// Check after reclaim...
+				if ((_currentOffset + space_needed) > _bankSize) {
+					DEBUG_ERROR("nvram full!\n");
+					goto exit;
 				}
 
-				remainingEntries->setObject(entryContainer);
-			} else if (varEntry->new_state == VAR_NEW_STATE_REMOVE) {
-				if (varEntry->existing_offset) {
-					DEBUG_INFO("marking entry at offset %#lx deleted\n", varEntry->existing_offset);
-					invalidateOffsets[invalidateOffsetIndex++] = varEntry->existing_offset;
-				} else {
-					DEBUG_INFO("No existing_offset , removing\n");
-				}
-
-				// not re-added to remainingEntries
+				DEBUG_INFO("%s AFTER reclaim new_state=%d, e_off=%#lx, c_off=%#x, uuid=%x%x, nameSize=%#x, dataSize=%#x\n",
+				    varEntry->header.name_data_buf,
+				    varEntry->new_state, varEntry->existing_offset, _currentOffset,
+				    varHeader->guid[0], varHeader->guid[1],
+				    varHeader->nameSize, varHeader->dataSize);
+			}
+
+			if (varEntry->existing_offset) {
+				// Mark existing entry as VAR_IN_DELETED_TRANSITION
+				state = varHeader->state & VAR_IN_DELETED_TRANSITION;
+				DEBUG_INFO("invalidating with state=%#x\n", state);
+
+				ret = _nvramController->write(varEntry->existing_offset + offsetof(struct v3_var_header, state), &state, sizeof(state));
+				require_noerr_action(ret, exit, DEBUG_ERROR("new state w fail, ret=%#x\n", ret));
+			}
+
+			varStartOffset = _currentOffset;
+			varEndOffset = _currentOffset;
+
+			// Append new entry as VAR_ADDED
+			varHeader->state = VAR_ADDED;
+
+			ret = _nvramController->write(varStartOffset, (uint8_t *)varHeader, variable_length(varHeader));
+			require_noerr_action(ret, exit, DEBUG_ERROR("variable write fail, ret=%#x\n", ret); );
+
+			varEndOffset += variable_length(varHeader);
+
+			if (varEntry->existing_offset) {
+				// Mark existing entry as VAR_DELETED
+				state = varHeader->state & VAR_DELETED & VAR_IN_DELETED_TRANSITION;
+
+				ret = _nvramController->write(varEntry->existing_offset + offsetof(struct v3_var_header, state), &state, sizeof(state));
+				require_noerr_action(ret, exit, DEBUG_ERROR("existing state w fail, ret=%#x\n", ret));
+			}
+
+			varEntry->existing_offset = varStartOffset;
+			varEntry->new_state = VAR_NEW_STATE_NONE;
+
+			_currentOffset = (uint32_t)varEndOffset;
+
+			remainingEntries->setObject(entryContainer);
+		} else if (varEntry->new_state == VAR_NEW_STATE_REMOVE) {
+			if (varEntry->existing_offset) {
+				DEBUG_INFO("marking entry at offset %#lx deleted\n", varEntry->existing_offset);
+
+				// Mark existing entry as VAR_IN_DELETED_TRANSITION
+				state = varHeader->state & VAR_DELETED & VAR_IN_DELETED_TRANSITION;
+
+				ret = _nvramController->write(varEntry->existing_offset + offsetof(struct v3_var_header, state), &state, sizeof(state));
+				require_noerr_action(ret, exit, DEBUG_ERROR("existing state w fail, ret=%#x\n", ret));
 			} else {
-				DEBUG_INFO("skipping\n");
-				remainingEntries->setObject(entryContainer);
-			}
-		}
-
-		if (appendBufferOffset > 0) {
-			// Write appendBuffer
-			DEBUG_INFO("Appending append buffer size=%#zx at offset=%#x\n", appendBufferOffset, _currentOffset);
-			ret = _nvramController->write(_currentOffset, appendBuffer, appendBufferOffset);
-			require_noerr_action(ret, unlock, DEBUG_ERROR("could not re-append, ret=%#x\n", ret));
-
-			_currentOffset += appendBufferOffset;
+				DEBUG_INFO("No existing, removing\n");
+			}
+
+			// not re-added to remainingEntries
 		} else {
-			DEBUG_INFO("No entries to append\n");
-		}
-
-		if (invalidateOffsetIndex > 0) {
-			// Invalidate Entries
-			for (unsigned int i = 0; i < invalidateOffsetIndex; i++) {
-				uint8_t state = VAR_ADDED & VAR_DELETED & VAR_IN_DELETED_TRANSITION;
-
-				ret = _nvramController->write(invalidateOffsets[i] + offsetof(struct v3_var_header, state), &state, sizeof(state));
-				require_noerr_action(ret, unlock, DEBUG_ERROR("unable to invalidate at offset %#zx, ret=%#x\n", invalidateOffsets[i], ret));
-				DEBUG_INFO("Invalidated entry at offset=%#zx\n", invalidateOffsets[i]);
-			}
-		} else {
-			DEBUG_INFO("No entries to invalidate\n");
-		}
-
-		_newData = false;
-		_varEntries.reset(remainingEntries.get(), OSRetain);
-unlock:
-		NVRAMRWUNLOCK(_variableLock);
-	} else {
-		// Will need to reclaim, rebuild store and write everything at once
-		NVRAMRWUNLOCK(_variableLock);
-		ret = reclaim();
-	}
+			DEBUG_INFO("skipping\n");
+			remainingEntries->setObject(entryContainer);
+		}
+	}
+
+	_varEntries.reset(remainingEntries.get(), OSRetain);
+
+	_newData = false;
 
 exit:
-	IOFreeData(appendBuffer, _bankSize);
-	IOFreeData(invalidateOffsets, invalidateOffsetsCount * sizeof(size_t));
-
 	return ret;
 }
 
@@ -1218,9 +1165,8 @@
 	require_action(_newData == true, exit, DEBUG_INFO("No _newData to sync\n"));
 	require_action(_bankSize != 0, exit, DEBUG_INFO("No nvram size info\n"));
 
-	block = (uint8_t *)IOMallocZeroData(_bankSize);
-
-	NVRAMREADLOCK(_variableLock);
+	block = (uint8_t *)IOMallocData(_bankSize);
+
 	remainingEntries = OSArray::withCapacity(_varEntries->getCapacity());
 
 	ret = _nvramController->select(next_bank);
@@ -1244,12 +1190,12 @@
 		varEntry = (struct nvram_v3_var_entry *)entryContainer->getBytesNoCopy();
 		varHeader = &varEntry->header;
 
+		varHeader->state = VAR_ADDED;
+
 		DEBUG_INFO("entry %u %s, new_state=%#x, e_offset=%#lx, state=%#x\n",
 		    i, varEntry->header.name_data_buf, varEntry->new_state, varEntry->existing_offset, varHeader->state);
 
 		if (varEntry->new_state != VAR_NEW_STATE_REMOVE) {
-			varHeader->state = VAR_ADDED;
-
 			memcpy(block + new_bank_offset, (uint8_t *)varHeader, variable_length(varHeader));
 
 			varEntry->existing_offset = new_bank_offset;
@@ -1262,17 +1208,12 @@
 		}
 	}
 
-	// 0xFF out the remaining space, this will allow banks to switch between append mode and
-	// block mode if ever needed
-	memset(block + new_bank_offset, 0xFF, _bankSize - (uint32_t)new_bank_offset);
-
 	ret = _nvramController->write(0, block, _bankSize);
 	verify_noerr_action(ret, DEBUG_ERROR("w fail, ret=%#x\n", ret));
 
 	_nvramController->sync();
 
 	_varEntries.reset(remainingEntries.get(), OSRetain);
-	NVRAMRWUNLOCK(_variableLock);
 
 	_newData = false;
 
@@ -1284,20 +1225,15 @@
 	return ret;
 }
 
-IOReturn
+bool
 IONVRAMV3Handler::sync(void)
 {
 	IOReturn ret;
-
-	NVRAMLOCK(_controllerLock);
 
 	if (_reload) {
 		ret = reloadInternal();
-		if (ret != kIOReturnSuccess) {
-			DEBUG_ERROR("Reload failed, ret=%#x, reclaiming\n", ret);
-			ret = reclaim();
-			require_noerr_action(ret, exit, DEBUG_ERROR("Reclaim recovery failed, ret=%#x\n", ret));
-		}
+		require_noerr_action(ret, exit, DEBUG_ERROR("Reload failed, ret=%#x", ret));
+
 		_reload = false;
 	}
 
@@ -1306,15 +1242,18 @@
 
 		if (ret != kIOReturnSuccess) {
 			ret = reclaim();
-			require_noerr_action(ret, exit, DEBUG_ERROR("Reclaim recovery failed, ret=%#x\n", ret));
+			require_noerr_action(ret, exit, DEBUG_ERROR("Reclaim recovery failed, ret=%#x", ret));
+
+			// Attempt to save again (will rewrite the variables still in APPEND) on the new bank
+			ret = syncRaw();
+			require_noerr_action(ret, exit, DEBUG_ERROR("syncRaw retry failed, ret=%#x", ret));
 		}
 	} else {
 		ret = syncBlock();
 	}
 
 exit:
-	NVRAMUNLOCK(_controllerLock);
-	return ret;
+	return ret == kIOReturnSuccess;
 }
 
 uint32_t
@@ -1339,12 +1278,6 @@
 IONVRAMV3Handler::getCommonUsed(void) const
 {
 	return _commonUsed;
-}
-
-bool
-IONVRAMV3Handler::getSystemPartitionActive(void) const
-{
-	return _systemSize != 0;
 }
 
 bool
@@ -1508,22 +1441,3 @@
 
 	return true;
 }
-
-IOReturn
-IONVRAMV3Handler::getVarDict(OSSharedPtr<OSDictionary> &varDictCopy)
-{
-	IOReturn ret = kIOReturnNotFound;
-
-	NVRAMREADLOCK(_variableLock);
-	if (_varDict) {
-		varDictCopy = OSDictionary::withDictionary(_varDict.get());
-		if (varDictCopy) {
-			if (OSDictionary::withCapacity(varDictCopy->getCount()) != nullptr) {
-				ret = kIOReturnSuccess;
-			}
-		}
-	}
-	NVRAMRWUNLOCK(_variableLock);
-
-	return ret;
-}