Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

plat/virtio: Fix vring_avail_event macro #1076

Merged
merged 3 commits into from Oct 3, 2023
Merged

plat/virtio: Fix vring_avail_event macro #1076

merged 3 commits into from Oct 3, 2023

Conversation

mschlumpp
Copy link
Member

Prerequisite checklist

  • Read the contribution guidelines regarding submitting new changes to the project;
  • Tested your changes against relevant architectures and platforms;
  • Ran the checkpatch.uk on your commit series before opening this PR;
  • Updated relevant documentation.

Base target

  • Architecture(s): N/A
  • Platform(s): kvm
  • Application(s): N/A

Additional configuration

Description of changes

This fixes the vring_avail_event to not cause miscompilations by violating the strict aliasing rules.

@mschlumpp mschlumpp requested review from a team as code owners August 30, 2023 15:40
@razvand razvand requested review from eduardvintila and rares-miculescu and removed request for a team September 8, 2023 20:54
@razvand razvand self-assigned this Sep 8, 2023
@razvand razvand added area/plat Unikraft Patform kind/quick-fix Issue is a quick fix plat/driver/virtio lang/c Issues or PRs to do with C/C++ labels Sep 8, 2023
@razvand razvand added this to the v0.15.0 (Pandora) milestone Sep 8, 2023
@mschlumpp
plat/virtio: Use correct type for vring_avail_event field
ec39a62 
This code was imported and this occurrence of the type was not adjusted when
the type name changed.

Signed-off-by: Marco Schlumpp <marco@unikraft.io>
@mschlumpp
include/uk: Introduce macro for may_alias attribute
9206e2d 
This attribute allows to opt-out of the standard C strict aliasing rule.

Signed-off-by: Marco Schlumpp <marco@unikraft.io>
@mschlumpp
plat/virtio: Use may_alias attribute for vring_avail_event
8ce5677 
The macro violates the strict aliasing rules. By using the newly introduced
may_alias attribute we can prevent miscompilations because of that.

Signed-off-by: Marco Schlumpp <marco@unikraft.io>
@mschlumpp mschlumpp force-pushed the mschlumpp/feature/virtio_may_alias branch from 069e0cf to 8ce5677 Compare September 12, 2023 07:39
@mschlumpp mschlumpp requested a review from a team as a code owner September 12, 2023 07:39
@unikraft-bot
Copy link
Member

⚠️ Checkpatch passed with warnings

Beep boop! I ran Unikraft's checkpatch.pl support script on your pull request but it encountered warnings:

SHA commit checkpatch
ec39a62 plat/virtio: Use correct type for vring_avail_event field ⚠️
9206e2d include/uk: Introduce macro for may_alias attribute ✅
8ce5677 plat/virtio: Use may_alias attribute for vring_avail_event ✅

Truncated logs starting from first warning ec39a62:

WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#6: 
This code was imported and this occurrence of the type was not adjusted when

total: 0 errors, 1 warnings, 8 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

/tmp/build/a53d4c5b/patches/0001-plat-virtio-Use-correct-type-for-vring_avail_event-f.patch has style problems, please review.

NOTE: Ignored message types: ASSIGN_IN_IF FILE_PATH_CHANGES NEW_TYPEDEFS OBSOLETE

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.

View complete logs | Learn more about Unikraft's coding style and contribution guidelines.

@unikraft-bot unikraft-bot added the area/include Part of include/uk label Sep 12, 2023
@razvand razvand removed the request for review from a team September 27, 2023 04:55
@razvand razvand requested review from Starnox and removed request for rares-miculescu September 27, 2023 04:55
michpappas pushed a commit to michpappas/unikraft that referenced this pull request Sep 27, 2023
@mschlumpp @michpappas
drivers/virtio: PR-unikraft#1076 Use correct type for vring_avail_eve…
7d794c5 
…nt field

Signed-off-by: Marco Schlumpp <marco@unikraft.io>
@michpappas michpappas mentioned this pull request Sep 27, 2023
Merged
4 tasks
michpappas pushed a commit to michpappas/unikraft that referenced this pull request Sep 27, 2023
@mschlumpp @michpappas
drivers/virtio: PR-unikraft#1076 Use correct type for vring_avail_eve…
0e868e7 
…nt field

Signed-off-by: Marco Schlumpp <marco@unikraft.io>
michpappas pushed a commit to michpappas/unikraft that referenced this pull request Sep 27, 2023
@mschlumpp @michpappas
drivers/virtio: PR-unikraft#1076 Use correct type for vring_avail_eve…
e8473af 
…nt field

Signed-off-by: Marco Schlumpp <marco@unikraft.io>
@razvand razvand requested review from rares-miculescu and removed request for Starnox October 1, 2023 01:41
eduardvintila
eduardvintila approved these changes Oct 1, 2023
View reviewed changes
Copy link
Member

@eduardvintila eduardvintila left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks!

Reviewed-by: Eduard Vintilă eduard.vintila47@gmail.com

@michpappas michpappas mentioned this pull request Oct 3, 2023
Closed
4 tasks
rares-miculescu
rares-miculescu approved these changes Oct 3, 2023
View reviewed changes
Copy link

@rares-miculescu rares-miculescu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good, thanks!

Reviewed-by: Rares Miculescu miculescur@gmail.com

razvand
razvand approved these changes Oct 3, 2023
View reviewed changes
Copy link
Contributor

@razvand razvand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved-by: Razvan Deaconescu razvand@unikraft.io

@razvand razvand merged commit bd661fa into unikraft:staging Oct 3, 2023
10 checks passed
razvand pushed a commit that referenced this pull request Oct 3, 2023
@mschlumpp @razvand
plat/virtio: Use correct type for vring_avail_event field
a342048 
This code was imported and this occurrence of the type was not adjusted when
the type name changed.

Signed-off-by: Marco Schlumpp <marco@unikraft.io>
Reviewed-by: Eduard Vintilă <eduard.vintila47@gmail.com>
Reviewed-by: Rares Miculescu <miculescur@gmail.com>
Approved-by: Razvan Deaconescu <razvand@unikraft.io>
GiHub-Closes: #1076
razvand pushed a commit that referenced this pull request Oct 3, 2023
@mschlumpp @razvand
include/uk: Introduce macro for may_alias attribute
cdd84bf 
This attribute allows to opt-out of the standard C strict aliasing rule.

Signed-off-by: Marco Schlumpp <marco@unikraft.io>
Reviewed-by: Eduard Vintilă <eduard.vintila47@gmail.com>
Reviewed-by: Rares Miculescu <miculescur@gmail.com>
Approved-by: Razvan Deaconescu <razvand@unikraft.io>
GiHub-Closes: #1076
razvand pushed a commit that referenced this pull request Oct 3, 2023
@mschlumpp @razvand
plat/virtio: Use may_alias attribute for vring_avail_event
4f7a6d9 
The macro violates the strict aliasing rules. By using the newly introduced
may_alias attribute we can prevent miscompilations because of that.

Signed-off-by: Marco Schlumpp <marco@unikraft.io>
Reviewed-by: Eduard Vintilă <eduard.vintila47@gmail.com>
Reviewed-by: Rares Miculescu <miculescur@gmail.com>
Approved-by: Razvan Deaconescu <razvand@unikraft.io>
GiHub-Closes: #1076
@mschlumpp mschlumpp deleted the mschlumpp/feature/virtio_may_alias branch October 6, 2023 08:32
razvand pushed a commit that referenced this pull request Oct 20, 2023
@mschlumpp @razvand
plat/virtio: Use correct type for vring_avail_event field
f01c4e2 
This code was imported and this occurrence of the type was not adjusted when
the type name changed.

Signed-off-by: Marco Schlumpp <marco@unikraft.io>
Reviewed-by: Eduard Vintilă <eduard.vintila47@gmail.com>
Reviewed-by: Rares Miculescu <miculescur@gmail.com>
Approved-by: Razvan Deaconescu <razvand@unikraft.io>
GiHub-Closes: #1076
razvand pushed a commit that referenced this pull request Oct 20, 2023
@mschlumpp @razvand
include/uk: Introduce macro for may_alias attribute
028aa63 
This attribute allows to opt-out of the standard C strict aliasing rule.

Signed-off-by: Marco Schlumpp <marco@unikraft.io>
Reviewed-by: Eduard Vintilă <eduard.vintila47@gmail.com>
Reviewed-by: Rares Miculescu <miculescur@gmail.com>
Approved-by: Razvan Deaconescu <razvand@unikraft.io>
GiHub-Closes: #1076
razvand pushed a commit that referenced this pull request Oct 20, 2023
@mschlumpp @razvand
plat/virtio: Use may_alias attribute for vring_avail_event
d276df7 
The macro violates the strict aliasing rules. By using the newly introduced
may_alias attribute we can prevent miscompilations because of that.

Signed-off-by: Marco Schlumpp <marco@unikraft.io>
Reviewed-by: Eduard Vintilă <eduard.vintila47@gmail.com>
Reviewed-by: Rares Miculescu <miculescur@gmail.com>
Approved-by: Razvan Deaconescu <razvand@unikraft.io>
GiHub-Closes: #1076
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@razvand razvand razvand approved these changes

@eduardvintila eduardvintila eduardvintila approved these changes

@rares-miculescu rares-miculescu rares-miculescu approved these changes

Assignees

@razvand razvand

Labels
area/include Part of include/uk area/plat Unikraft Patform kind/quick-fix Issue is a quick fix lang/c Issues or PRs to do with C/C++ plat/driver/virtio
Projects
Unikraft Core Roadmap
Status: ✅ Done!
Release 0.15
Status: Done
Milestone
v0.15.0 (Pandora)
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@mschlumpp @unikraft-bot @razvand @eduardvintila @rares-miculescu @nderjung