On 8/23/17 6:21 PM, Laszlo Ersek wrote: > (1) Please replace "VIRITO_F_IOMMU_PLATFORM" with > "VIRTIO_F_IOMMU_PLATFORM" in the subjects of the remaining patches > (including this one).
I will fix it in v4. > On 08/23/17 14:22, Brijesh Singh wrote: >> In previous patches, we have implemented IOMMU-like member functions in >> VIRTIO_DEVICE_PROTOCOL to translate the physical address to bus address >> and virtio drivers are updated to use those member functions. We do not >> need to do anything special when VIRTIO_F_IOMMU_PLATFORM bit is present >> hence treat it in parallel with VIRTIO_F_VERSION_1. >> >> Cc: Ard Biesheuvel <[email protected]> >> Cc: Jordan Justen <[email protected]> >> Cc: Tom Lendacky <[email protected]> >> Cc: Laszlo Ersek <[email protected]> >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Brijesh Singh <[email protected]> >> --- >> OvmfPkg/VirtioRngDxe/VirtioRng.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/OvmfPkg/VirtioRngDxe/VirtioRng.c >> b/OvmfPkg/VirtioRngDxe/VirtioRng.c >> index 59f32d343179..32512d882f7d 100644 >> --- a/OvmfPkg/VirtioRngDxe/VirtioRng.c >> +++ b/OvmfPkg/VirtioRngDxe/VirtioRng.c >> @@ -278,7 +278,7 @@ VirtioRngInit ( >> goto Failed; >> } >> >> - Features &= VIRTIO_F_VERSION_1; >> + Features &= VIRTIO_F_VERSION_1 | VIRTIO_F_IOMMU_PLATFORM; >> >> // >> // In virtio-1.0, feature negotiation is expected to complete before queue >> @@ -359,7 +359,7 @@ VirtioRngInit ( >> // step 5 -- Report understood features and guest-tuneables. >> // >> if (Dev->VirtIo->Revision < VIRTIO_SPEC_REVISION (1, 0, 0)) { >> - Features &= ~(UINT64)VIRTIO_F_VERSION_1; >> + Features &= ~(UINT64)VIRTIO_F_VERSION_1 | VIRTIO_F_IOMMU_PLATFORM; > While all of the VIRTIO_F_VERSION_1 locations in this driver are covered > now, this change is invalid: > > The bitwise-complement operator "~" has stronger binding than the > bitwise-or operator "|". The cast operator "(type)" also binds more > strongly than the bitwise-or operator "|". > > The purpose of the above assignment is to clear both of the listed bits. > Therefore, > > (2) parentheses are necessary tightly around the bitwise-or operator, > like I suggested in > <[email protected]">http://mid.mail-archive.com/[email protected]>, > point (4): > > Features &= ~(UINT64)(VIRTIO_F_VERSION_1 | VIRTIO_F_IOMMU_PLATFORM); Agreed, thanks. I will fix it in v4. > I think this comment too applies to the rest of the patches. > > Thanks, > Laszlo > >> Status = Dev->VirtIo->SetGuestFeatures (Dev->VirtIo, Features); >> if (EFI_ERROR (Status)) { >> goto UnmapQueue; >> _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
