| Message ID | 1585538712-28300-1-git-send-email-hang.yuan@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] target/i386: HAX: Enable ROM/ROM device memory region support | expand |
Looks good to me. Reviewed-by: Colin Xu <colin.xu@intel.com> On 2020-03-30 11:25, hang.yuan@linux.intel.com wrote: > From: Hang Yuan <hang.yuan@linux.intel.com> > > Add ROM and ROM device memory region support in HAX. Their memory region is > read only and write access will generate EPT violation. The violation will be > handled in the HAX kernel with the following patch. > > https://github.com/intel/haxm/commit/33ceea09a1655fca12c47f1e112b1d269357ff28 > > v2: fix coding style problems > > Signed-off-by: Hang Yuan <hang.yuan@linux.intel.com> > --- > target/i386/hax-mem.c | 11 ++++------- > 1 file changed, 4 insertions(+), 7 deletions(-) > > diff --git a/target/i386/hax-mem.c b/target/i386/hax-mem.c > index 6bb5a24917..a8bfd37977 100644 > --- a/target/i386/hax-mem.c > +++ b/target/i386/hax-mem.c > @@ -175,13 +175,10 @@ static void hax_process_section(MemoryRegionSection *section, uint8_t flags) > uint64_t host_va; > uint32_t max_mapping_size; > > - /* We only care about RAM and ROM regions */ > - if (!memory_region_is_ram(mr)) { > - if (memory_region_is_romd(mr)) { > - /* HAXM kernel module does not support ROMD yet */ > - warn_report("Ignoring ROMD region 0x%016" PRIx64 "->0x%016" PRIx64, > - start_pa, start_pa + size); > - } > + /* We only care about RAM/ROM regions and ROM device */ > + if (memory_region_is_rom(mr) || (memory_region_is_romd(mr))) { > + flags |= HAX_RAM_INFO_ROM; > + } else if (!memory_region_is_ram(mr)) { > return; > } >
Hi Paolo, Would you please queue this one? -- Best Regards, Colin Xu On Mon, 30 Mar 2020, Colin Xu wrote: > Looks good to me. > > Reviewed-by: Colin Xu <colin.xu@intel.com> > > On 2020-03-30 11:25, hang.yuan@linux.intel.com wrote: >> From: Hang Yuan <hang.yuan@linux.intel.com> >> >> Add ROM and ROM device memory region support in HAX. Their memory region is >> read only and write access will generate EPT violation. The violation will be >> handled in the HAX kernel with the following patch. >> >> https://github.com/intel/haxm/commit/33ceea09a1655fca12c47f1e112b1d269357ff28 >> >> v2: fix coding style problems >> >> Signed-off-by: Hang Yuan <hang.yuan@linux.intel.com> >> --- >> target/i386/hax-mem.c | 11 ++++------- >> 1 file changed, 4 insertions(+), 7 deletions(-) >> >> diff --git a/target/i386/hax-mem.c b/target/i386/hax-mem.c >> index 6bb5a24917..a8bfd37977 100644 >> --- a/target/i386/hax-mem.c >> +++ b/target/i386/hax-mem.c >> @@ -175,13 +175,10 @@ static void hax_process_section(MemoryRegionSection *section, uint8_t flags) >> uint64_t host_va; >> uint32_t max_mapping_size; >> >> - /* We only care about RAM and ROM regions */ >> - if (!memory_region_is_ram(mr)) { >> - if (memory_region_is_romd(mr)) { >> - /* HAXM kernel module does not support ROMD yet */ >> - warn_report("Ignoring ROMD region 0x%016" PRIx64 "->0x%016" PRIx64, >> - start_pa, start_pa + size); >> - } >> + /* We only care about RAM/ROM regions and ROM device */ >> + if (memory_region_is_rom(mr) || (memory_region_is_romd(mr))) { >> + flags |= HAX_RAM_INFO_ROM; >> + } else if (!memory_region_is_ram(mr)) { >> return; >> } >> > > -- > Best Regards, > Colin Xu > > >
On 4/28/20 4:45 AM, Colin Xu wrote: > > Hi Paolo, > > Would you please queue this one? > -- > Best Regards, > Colin Xu > > On Mon, 30 Mar 2020, Colin Xu wrote: > >> Looks good to me. >> >> Reviewed-by: Colin Xu <colin.xu@intel.com> >> >> On 2020-03-30 11:25, hang.yuan@linux.intel.com wrote: >>> From: Hang Yuan <hang.yuan@linux.intel.com> >>> >>> Add ROM and ROM device memory region support in HAX. Their memory >>> region is >>> read only and write access will generate EPT violation. The violation >>> will be >>> handled in the HAX kernel with the following patch. "will be"? This patch is 10 months old. >>> >>> https://github.com/intel/haxm/commit/33ceea09a1655fca12c47f1e112b1d269357ff28 >>> >>> >>> v2: fix coding style problems ^ This line goes ... >>> >>> Signed-off-by: Hang Yuan <hang.yuan@linux.intel.com> >>> --- ... here after the '---'. >>> target/i386/hax-mem.c | 11 ++++------- >>> 1 file changed, 4 insertions(+), 7 deletions(-) >>> >>> diff --git a/target/i386/hax-mem.c b/target/i386/hax-mem.c >>> index 6bb5a24917..a8bfd37977 100644 >>> --- a/target/i386/hax-mem.c >>> +++ b/target/i386/hax-mem.c >>> @@ -175,13 +175,10 @@ static void >>> hax_process_section(MemoryRegionSection *section, uint8_t flags) >>> uint64_t host_va; >>> uint32_t max_mapping_size; >>> >>> - /* We only care about RAM and ROM regions */ >>> - if (!memory_region_is_ram(mr)) { >>> - if (memory_region_is_romd(mr)) { >>> - /* HAXM kernel module does not support ROMD yet */ >>> - warn_report("Ignoring ROMD region 0x%016" PRIx64 >>> "->0x%016" PRIx64, >>> - start_pa, start_pa + size); Don't you need to check for some kmod version before removing this check? >>> - } >>> + /* We only care about RAM/ROM regions and ROM device */ >>> + if (memory_region_is_rom(mr) || (memory_region_is_romd(mr))) { Redundant parenthesis. >>> + flags |= HAX_RAM_INFO_ROM; >>> + } else if (!memory_region_is_ram(mr)) { >>> return; >>> } If you move the 'if (RAM) return' first, the code becomes easier to review. >>> >> >> -- >> Best Regards, >> Colin Xu >> >> >> >
diff --git a/target/i386/hax-mem.c b/target/i386/hax-mem.c index 6bb5a24917..a8bfd37977 100644 --- a/target/i386/hax-mem.c +++ b/target/i386/hax-mem.c @@ -175,13 +175,10 @@ static void hax_process_section(MemoryRegionSection *section, uint8_t flags) uint64_t host_va; uint32_t max_mapping_size; - /* We only care about RAM and ROM regions */ - if (!memory_region_is_ram(mr)) { - if (memory_region_is_romd(mr)) { - /* HAXM kernel module does not support ROMD yet */ - warn_report("Ignoring ROMD region 0x%016" PRIx64 "->0x%016" PRIx64, - start_pa, start_pa + size); - } + /* We only care about RAM/ROM regions and ROM device */ + if (memory_region_is_rom(mr) || (memory_region_is_romd(mr))) { + flags |= HAX_RAM_INFO_ROM; + } else if (!memory_region_is_ram(mr)) { return; }