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

Conversation

raphaelning
Copy link
Contributor

Add a top-level license file detectable by GitHub. It is created
using GitHub's standard BSD 3-clause license template.

Add a top-level license file detectable by GitHub. It is created
using GitHub's standard BSD 3-clause license template.
@raphaelning raphaelning requested a review from wcwang November 14, 2017 09:46
Copy link
Contributor

@stweil stweil left a comment

Choose a reason for hiding this comment

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

@raphaelning
Copy link
Contributor Author

It turns out one of the source files (core/page_walker.c) has a different license (Apache 2.0) due to historical reasons, so we have to find another LICENSE file template. My colleague is working on that.

@stweil
Copy link
Contributor

stweil commented Nov 15, 2017

Is it possible to add the BSD 3-clause license to core/page_walker.c (and optionally remove the Apache 2.0 license)? If all license holders (Intel Corporation, others?) agree, that would be a simple solution.

@raphaelning
Copy link
Contributor Author

Makes sense. I believe the only license holder is Intel, since I was told the code was taken from another Intel software project. I'll check with my colleague to see if we can just replace the license for this file.

Speaking of licenses, include/hax_interface.h and include/vcpu_state.h are basically the same code as target/i386/hax-interface.h in the QEMU tree, but the QEMU file has a GPLv2 license. I'm not sure if this inconsistency is an issue. Should we add the BSD license to the QEMU file, and perhaps remove the GPLv2 license?

@stweil
Copy link
Contributor

stweil commented Nov 15, 2017

Adding the BSD license looks logical for me.

I suggest to send a git patch for target/i386/hax-interface.h which adds the BSD license to the qemu-devel mailing list. Then we can discuss there whether it is better to have two licenses for that file or whether it is fine to have only the BSD license.

@mattl
Copy link

mattl commented Nov 15, 2017

Glad to see this is happening, I was going to file an issue about this :)

@raphaelning
Copy link
Contributor Author

Thanks, I'll make the patch and send it to qemu-devel.

As for core/page_walker.c, the advice I got from my colleagues is to relicense it to 3-clause BSD. There is an internal approval process we need to go through, but we think it can be done by tomorrow. Then I'll come back and merge this change.

@raphaelning
Copy link
Contributor Author

core/page_walker.c has been relicensed to 3-clause BSD. Merging this patch now.

@raphaelning raphaelning merged commit 4b4a879 into master Nov 20, 2017
@raphaelning raphaelning deleted the top-level-license-file branch November 20, 2017 03:10
raphaelning added a commit that referenced this pull request Mar 25, 2019
Android Emulator may dynamically create and destroy temporary
memory mappings at guest runtime for certain rendering tasks via
hax_user_backed_ram_map() and hax_user_backed_ram_unmap()
($AOSP/external/qemu/target/i386/hax-mem.c), e.g.:

 hax_user_backed_ram_map() #1
 1.1) ADD_RAMBLOCK (#1):   HVA 0x14e070000..0x16e070000
 1.2) SET_RAM2 (map #1):   GPA 0x7dffff000..0x7fffff000 =>
                           HVA 0x14e070000..0x16e070000
 hax_user_backed_ram_unmap() #1
 1.3) SET_RAM2 (unmap #1): GPA 0x7dffff000..0x7fffff000
 hax_user_backed_ram_map() #2
 2.1) ADD_RAMBLOCK: (#2):  HVA 0x14de70000..0x16de70000

The second ADD_RAMBLOCK call fails, because its HVA range overlaps
with that of the first ADD_RAMBLOCK call.

The problem is that the "map" step creates a RAM block, but the
"unmap" step doesn't destroy it. Instead of adding a DEL_RAMBLOCK
ioctl, simply exempt the caller from calling ADD_RAMBLOCK in the
first place:

 - Introduce a new hax_memslot flag for "stand-alone" mappings.
 - Remove the ADD_RAMBLOCK call from hax_user_backed_ram_map().
   Instead, call SET_RAM2 with the new flag. (This will be done on
   the Android Emulator side.)
 - Internally, SET_RAM2 creates a stand-alone RAM block for each
   stand-alone mapping.
 - When the stand-alone mapping is unmapped, the reference count
   of the corresponding stand-alone RAM block will hit 0, which
   allows SET_RAM2 to destroy this temporary RAM block.

Signed-off-by: Yu Ning <yu.ning@intel.com>
raphaelning added a commit that referenced this pull request Mar 25, 2019
Android Emulator may dynamically create and destroy temporary
memory mappings at guest runtime for certain rendering tasks via
hax_user_backed_ram_map() and hax_user_backed_ram_unmap()
($AOSP/external/qemu/target/i386/hax-mem.c), e.g.:

 hax_user_backed_ram_map() <1>
 1.1) ADD_RAMBLOCK (#1):   HVA 0x14e070000..0x16e070000
 1.2) SET_RAM2 (map #1):   GPA 0x7dffff000..0x7fffff000 =>
                           HVA 0x14e070000..0x16e070000
 hax_user_backed_ram_unmap() <1>
 1.3) SET_RAM2 (unmap #1): GPA 0x7dffff000..0x7fffff000
 hax_user_backed_ram_map() <2>
 2.1) ADD_RAMBLOCK: (#2):  HVA 0x14de70000..0x16de70000

The second ADD_RAMBLOCK call fails, because its HVA range overlaps
with that of the first ADD_RAMBLOCK call.

The problem is that the "map" step creates a RAM block, but the
"unmap" step doesn't destroy it. Instead of adding a DEL_RAMBLOCK
ioctl, simply exempt the caller from calling ADD_RAMBLOCK in the
first place:

 - Introduce a new hax_memslot flag for "stand-alone" mappings,
   along with a new capability flag for this API change.
 - Remove the ADD_RAMBLOCK call from hax_user_backed_ram_map().
   Instead, call SET_RAM2 with the new flag. (This will be done on
   the Android Emulator side.)
 - Internally, SET_RAM2 creates a stand-alone RAM block for each
   stand-alone mapping.
 - When the stand-alone mapping is unmapped, the reference count
   of the corresponding stand-alone RAM block will hit 0, which
   allows SET_RAM2 to destroy this temporary RAM block.

+ Replace HAX_RAM_INFO_xxx with HAX_MEMSLOT_xxx in code that is
  not directly in touch with user space.

Signed-off-by: Yu Ning <yu.ning@intel.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants