Skip to content
This repository was archived by the owner on Jan 28, 2023. It is now read-only.

Add rom device support #213

Merged
merged 1 commit into from
Jun 21, 2019
Merged

Add rom device support #213

merged 1 commit into from
Jun 21, 2019

Conversation

hyuan3
Copy link
Contributor

@hyuan3 hyuan3 commented Jun 18, 2019

ROM device is read-only. Write operation will generte EPT violation
and then will be handled as mmio write.

Signed-off-by: Hang Yuan hang.yuan@intel.com

wcwang
wcwang reviewed Jun 18, 2019
View reviewed changes
core/ept2.c Outdated
if (combined_perm != HAX_EPT_PERM_NONE) {
if ((qual.raw & HAX_EPT_ACC_W) && !(combined_perm & HAX_EPT_PERM_W) &&
(slot->flags == HAX_MEMSLOT_READONLY)) {
// write to rom device
Copy link
Contributor

Choose a reason for hiding this comment

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

Write to ROM device

core/include/ept2.h
@@ -45,9 +45,14 @@
#define HAX_EPT_TABLE_SHIFT 9
#define HAX_EPT_TABLE_SIZE (1 << HAX_EPT_TABLE_SHIFT)

#define HAX_EPT_ACC_R 0x1
Copy link
Contributor

Choose a reason for hiding this comment

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

How about 'HAX_EPT_ACCESS_R'? Why define read access as 0x1 rather than 0x4?

wcwang
wcwang reviewed Jun 18, 2019
View reviewed changes
Copy link
Contributor

@wcwang wcwang left a comment •
edited
Loading

Choose a reason for hiding this comment

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

As ROM device is read-only, the write operation on ROM device will
generate EPT violation and then it will be handled as MMIO write,
which will cause an access error and result in VM launch failure.

@hyuan3 hyuan3 force-pushed the master branch 2 times, most recently from 9aba437 to 33ceea0 Compare June 19, 2019 06:30
@HaxmCI HaxmCI added CI:Build Pass CI:Build Pass CI:Mac Test Pass CI:Mac Test Pass labels Jun 19, 2019
@hyuan3
Add ROM device support
33ceea0 
ROM device is read-only. Its read operation will pass through. Its
Write operation will be handled as MMIO. Don't dinguish ROM and ROM
device exactly as no side impact.

Signed-off-by: Hang Yuan <hang.yuan@intel.com>
@wcwang wcwang self-requested a review June 21, 2019 09:29
@wcwang wcwang merged commit 7214275 into intel:master Jun 21, 2019
@wcwang wcwang removed their request for review June 21, 2019 09:30
@oliverhbailey
Copy link

Well Intel,
You developer here isn't getting the job done. You cannot specify any ROM for OVML and boot a VM with QEMU. This developer states he signed off on this June 2019, and everyone on the project who hasn't contributed in the past year, gives lip service. If this problem isn't fixed, its time to contact the escalation team, which I will do this week. There are several people claiming to be working on this project, but only one has contributed, and two just talk. Please put some fire under these developers to get this fixed. Intel is a member of the UEFI team!!!

@wcwang
Copy link
Contributor

wcwang commented Apr 7, 2020

Thanks for your comments. Could you provide the detailed command line to specify the ROM for OVMF so that we can clarify the requirements further? As the main development is targeted to Android Emulator, maybe we could not cover enough test cases for QEMU. Sorry for bring you inconvenience.

@oliverhbailey
Copy link

oliverhbailey commented Apr 7, 2020 via email

@hyuan3
Copy link
Contributor Author

hyuan3 commented Apr 7, 2020

Well Intel,
You developer here isn't getting the job done. You cannot specify any ROM for OVML and boot a VM with QEMU. This developer states he signed off on this June 2019, and everyone on the project who hasn't contributed in the past year, gives lip service. If this problem isn't fixed, its time to contact the escalation team, which I will do this week. There are several people claiming to be working on this project, but only one has contributed, and two just talk. Please put some fire under these developers to get this fixed. Intel is a member of the UEFI team!!!

Hi Oliver, "Intel is a member of the UEFI team" doesn't mandate all Intel's SW should support UEFI in default. As mentioned in @wcwang's comment, the initiative of HAXM is to support Android Emulator. So OVMF was not supported at the beginning. HAXM doesn't claim and also is impossible to support all Intel hardware features or all specifications Intel joined. Hope you can understand.

Thanks,
Henry

@oliverhbailey
Copy link

oliverhbailey commented Apr 7, 2020 via email

@hyuan3
Copy link
Contributor Author

hyuan3 commented Apr 8, 2020

Hi Oliver,

I'm an Intel employee working on HAXM as well as other projects. In general, HAXM is a small project and is not intended to support all technologies Intel has. So it's possible that some features are not there. On this specific UEFI issue, do you apply the Qemu patch for HAXM ROMD support which I pasted in another issue discussion thread? From our validation, it can support OVMF boot up.

Regards,
Henry

@oliverhbailey
Copy link

oliverhbailey commented Apr 8, 2020 via email

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@wcwang wcwang

Assignees
No one assigned
Labels
CI:Build Pass CI:Build Pass CI:Mac Test Pass CI:Mac Test Pass
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@hyuan3 @oliverhbailey @wcwang @HaxmCI