On 08/26/15 11:02, Michael Zimmermann wrote: > 1/2: sry, I'll correct that(if we'll proceed) > 3: I've ported EDK2 to a Smartphone. such devices don't have NOR/Flash > at all. They just have a MMC device. Since I hate duplicate code I just > included the OVMF packages in my DSC. If you say it's obsolete I > probably should copy this code to my device's platform package?
Since you are (apparently) not upstreaming the rest of your port, I'd rather not see OvmfPkg code (deprecated or otherwise) changed incompatibly, without other in-tree patches that justify such a change. (Re deprecated or not -- the fact that the code is deprecated makes it easier to argue for changing the code (-> lower impact, lower significance of incompatibility); on the other hand, deprecation makes it much harder to allocate review bandwidth.) So yes, I believe this change should be part of your private port. Thanks Laszlo > > On Wed, Aug 26, 2015 at 10:52 AM, Laszlo Ersek <[email protected] > <mailto:[email protected]>> wrote: > > On 08/21/15 20:28, Michael Zimmermann wrote: > > I'm using this lib on ARM and it generates unaligned access in it's > current > > state due to the heavy use of pointer casts. > > This changes the binary format so all previously saved data will be > lost. > > > > diff --git > a/OvmfPkg/Library/SerializeVariablesLib/SerializeVariablesLib.c > > b/OvmfPkg/Library/SerializeVariablesLib/SerializeVariablesLib.c > > index 6822c5c..f6c92c1 100644 > > (1) malformed patch > > (2) subject line doesn't spell out package & component name (should > start with OvmfPkg: SerializeVariablesLib: ...) > > (3) what are you using this for on ARM? This library had been meant as a > temporary stop-gap for OVMF until OVMF got flash (and therefore real > non-volatile variable support). In general the way forward here should > be removing this infrastructure from OvmfPkg, not adopting it elsewhere. > > Thanks > Laszlo > > > --- a/OvmfPkg/Library/SerializeVariablesLib/SerializeVariablesLib.c > > +++ b/OvmfPkg/Library/SerializeVariablesLib/SerializeVariablesLib.c > > @@ -80,13 +80,13 @@ UnpackVariableFromBuffer ( > > } > > > > *Name = (CHAR16*) (BytePtr + Offset); > > - Offset = Offset + *(UINT32*)BytePtr; > > + Offset = Offset + ALIGN_VALUE(*NameSize, sizeof(UINT32)); > > if (Offset > MaxSize) { > > return EFI_INVALID_PARAMETER; > > } > > > > *Guid = (EFI_GUID*) (BytePtr + Offset); > > - Offset = Offset + sizeof (EFI_GUID); > > + Offset = Offset + ALIGN_VALUE(sizeof (EFI_GUID), sizeof(UINT32)); > > if (Offset > MaxSize) { > > return EFI_INVALID_PARAMETER; > > } > > @@ -104,7 +104,7 @@ UnpackVariableFromBuffer ( > > } > > > > *Data = (VOID*) (BytePtr + Offset); > > - Offset = Offset + *DataSize; > > + Offset = Offset + ALIGN_VALUE(*DataSize, sizeof(UINT32)); > > if (Offset > MaxSize) { > > return EFI_INVALID_PARAMETER; > > } > > @@ -284,26 +284,26 @@ IterateVariablesCallbackSetSystemVariable ( > > IN VOID *Data > > ) > > { > > - EFI_STATUS Status; > > - STATIC CONST UINT32 AuthMask = > > - EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS | > > - > EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS; > > - > > - Status = gRT->SetVariable ( > > - VariableName, > > - VendorGuid, > > - Attributes, > > - DataSize, > > - Data > > - ); > > - > > - if (Status == EFI_SECURITY_VIOLATION && (Attributes & AuthMask) > != 0) { > > - DEBUG ((DEBUG_WARN, "%a: setting authenticated variable \"%s\" " > > - "failed with EFI_SECURITY_VIOLATION, ignoring\n", > __FUNCTION__, > > - VariableName)); > > - Status = EFI_SUCCESS; > > - } > > - return Status; > > + EFI_STATUS Status; > > + STATIC CONST UINT32 AuthMask = > > + EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS | > > + > EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS; > > + > > + Status = gRT->SetVariable ( > > + VariableName, > > + VendorGuid, > > + Attributes, > > + DataSize, > > + Data > > + ); > > + > > + if (Status == EFI_SECURITY_VIOLATION && (Attributes & AuthMask) > != 0) { > > + DEBUG ((DEBUG_WARN, "%a: setting authenticated variable \"%s\" " > > + "failed with EFI_SECURITY_VIOLATION, ignoring\n", > __FUNCTION__, > > + VariableName)); > > + Status = EFI_SUCCESS; > > + } > > + return Status; > > } > > > > > > @@ -366,7 +366,7 @@ AppendToBuffer ( > > Size > > ); > > > > - Instance->DataSize = NewSize; > > + Instance->DataSize = ALIGN_VALUE(NewSize, sizeof(UINT32)); > > } > > > > > > @@ -764,12 +764,12 @@ SerializeVariablesAddVariable ( > > SerializedNameSize = (UINT32) StrSize (VariableName); > > > > SerializedSize = > > - sizeof (SerializedNameSize) + > > - SerializedNameSize + > > - sizeof (*VendorGuid) + > > - sizeof (Attributes) + > > - sizeof (SerializedDataSize) + > > - DataSize; > > + ALIGN_VALUE(sizeof (SerializedNameSize), sizeof(UINT32)) + > > + ALIGN_VALUE(SerializedNameSize, sizeof(UINT32)) + > > + ALIGN_VALUE(sizeof (*VendorGuid), sizeof(UINT32)) + > > + ALIGN_VALUE(sizeof (Attributes), sizeof(UINT32)) + > > + ALIGN_VALUE(sizeof (SerializedDataSize), sizeof(UINT32)) + > > + ALIGN_VALUE(DataSize, sizeof(UINT32)); > > > > Status = EnsureExtraBufferSpace ( > > Instance, > > _______________________________________________ > > edk2-devel mailing list > > [email protected] <mailto:[email protected]> > > https://lists.01.org/mailman/listinfo/edk2-devel > > > > _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
