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.

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

Add AMD SEV-SNP vm support #3682

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

zhencliu
Copy link
Contributor

No description provided.

@zhencliu zhencliu force-pushed the snp branch 4 times, most recently from 12fe5e9 to 0ca6462 Compare May 11, 2023 02:04
@bssrikanth
Copy link
Contributor

bssrikanth commented Nov 21, 2023

Hello @zhencliu , I wanted to give this a try. Is it a recent version which is pushed here in this draft or do you have any recent updates which you are yet to push into this draft which resolves conflicts with recent master?

@zhencliu
Copy link
Contributor Author

Hello @zhencliu , I wanted to give this a try. Is it a recent version which is pushed here in this draft or do you have any recent updates which you are yet to push into this draft which resolves conflicts with recent master?

Hi, thanks for looking into this, this patch was not updated since I pushed it, it's out of date, the reason is SNP is not yet supported even in upstream, so we still don't have a final qemu command line which can be running a SNP VM, the options/params are TBD, talked with VT maintainers, we'll wait till the qemu upstream is ready.

bssrikanth pushed a commit to bssrikanth/avocado-vt that referenced this pull request Nov 30, 2023
Fix SNP mem allocation issue ontop
of SNP draft PR avocado-framework#3682

Signed-off-by: Srikanth Aithal <[email protected]>
bssrikanth pushed a commit to bssrikanth/avocado-vt that referenced this pull request Nov 30, 2023
Fix SNP mem allocation issue ontop
of SNP draft PR avocado-framework#3682 + AMDSEV fix

Signed-off-by: Srikanth Aithal <[email protected]>
@zhencliu zhencliu force-pushed the snp branch 2 times, most recently from bdb8008 to 77d17f1 Compare June 7, 2024 10:22
@zhencliu zhencliu marked this pull request as ready for review August 14, 2024 07:26
@zixi-chen
Copy link
Contributor

@zhencliu after #3928 merged, does this patch need to update?

qemu cmdline sample:
    -object sev-snp-guest,id=sev0, \
    cbitpos=51,reduced-phys-bits=1, \
    policy=0x30000, \
    id-block=YWFhYWFhYWFhYWFhYWFhCg==, \
    id-auth=CxHK/OKLkXGn/KpAC7Wl1FSiisWDbGTEKz..., \
    auth-key-enabled=on, \
    host-data=LNkCWBRC5CcdGXirbNUV1OrsR28s..., \
    guest-visible-workarounds=AA==, \

Signed-off-by: Zhenchao Liu <[email protected]>
@zhencliu
Copy link
Contributor Author

@zhencliu after #3928 merged, does this patch need to update?

Hi @zixi-chen , conflict resolved, thanks.

@bssrikanth
Copy link
Contributor

@zhencliu I was testing this PR, when SNP guest boots I also see memory-backend-ram getting assigned by default in addition to the memory-backend-memfd object. When further debugged I see this place

params.setdefault("backend", "memory-backend-ram")
assigns a memory-backend-ram object by default. IMO if users passes memory-backend-memfd we can skip adding this default memory-backend-ram object. Please let me know your thoughts.

@zhencliu
Copy link
Contributor Author

zhencliu commented Oct 24, 2024

@zhencliu I was testing this PR,

Thanks.

when SNP guest boots I also see memory-backend-ram getting assigned by default in addition to the memory-backend-memfd object. When further debugged I see this place

params.setdefault("backend", "memory-backend-ram")

assigns a memory-backend-ram object by default.

The default memory-backend-ram is used only when we don't assign any value

IMO if users passes memory-backend-memfd we can skip adding this default memory- backend-ram object. Please let me know your thoughts.

Yes, may I ask how you pass memory-backend-memfd ? Actually we have a cartesian param "vm_mem_backend" to be used for assign the memory backend, you can set it as
vm_mem_backend = memory-backend-memfd
and then have a try, then memory-backend-ram will not be used

@bssrikanth
Copy link
Contributor

@zhencliu I was testing this PR,

Thanks.

when SNP guest boots I also see memory-backend-ram getting assigned by default in addition to the memory-backend-memfd object. When further debugged I see this place

params.setdefault("backend", "memory-backend-ram")

assigns a memory-backend-ram object by default.

The default memory-backend-ram is used only when we don't assign any value

IMO if users passes memory-backend-memfd we can skip adding this default memory- backend-ram object. Please let me know your thoughts.

Yes, may I ask how you pass memory-backend-memfd ? Actually we have a cartesian param "vm_mem_backend" to be used for assign the memory backend, you can set it as vm_mem_backend = memory-backend-memfd and then have a try, then memory-backend-ram will not be used

Thanks for getting back.

I was using backend_mem=memory-backend-memfd, now I ran with vm_mem_backend = memory-backend-memfd -> it did not make any difference I continue to see memory-backend-ram during vm run.

I am listing configs I use from a memory and vm type below, please let me know your config:

use_mem = no
vm_secure_guest_type = snp
vm_sev_cbitpos = 51
vm_sev_reduced_phys_bits = 1
vm_mem_backend = memory-backend-memfd
mem_devs = "upm0"
vm_sev_upm_devs = "upm0"
vm_sev_upm_memdev_upm0 = upm0
size_mem_upm0 = 4096M

@zhencliu
Copy link
Contributor Author

zhencliu commented Oct 24, 2024

@zhencliu I was testing this PR,

Thanks.

when SNP guest boots I also see memory-backend-ram getting assigned by default in addition to the memory-backend-memfd object. When further debugged I see this place

params.setdefault("backend", "memory-backend-ram")

assigns a memory-backend-ram object by default.

The default memory-backend-ram is used only when we don't assign any value

IMO if users passes memory-backend-memfd we can skip adding this default memory- backend-ram object. Please let me know your thoughts.

Yes, may I ask how you pass memory-backend-memfd ? Actually we have a cartesian param "vm_mem_backend" to be used for assign the memory backend, you can set it as vm_mem_backend = memory-backend-memfd and then have a try, then memory-backend-ram will not be used

Thanks for getting back.

I was using backend_mem=memory-backend-memfd, now I ran with vm_mem_backend = memory-backend-memfd -> it did not make any difference I continue to see memory-backend-ram during vm run.

I am listing configs I use from a memory and vm type below, please let me know your config:

use_mem = no
vm_secure_guest_type = snp
vm_sev_cbitpos = 51
vm_sev_reduced_phys_bits = 1
vm_mem_backend = memory-backend-memfd
mem_devs = "upm0"
vm_sev_upm_devs = "upm0"
vm_sev_upm_memdev_upm0 = upm0
size_mem_upm0 = 4096M

@zhencliu I was testing this PR,

Thanks.

when SNP guest boots I also see memory-backend-ram getting assigned by default in addition to the memory-backend-memfd object. When further debugged I see this place

params.setdefault("backend", "memory-backend-ram")

assigns a memory-backend-ram object by default.

The default memory-backend-ram is used only when we don't assign any value

IMO if users passes memory-backend-memfd we can skip adding this default memory- backend-ram object. Please let me know your thoughts.

Yes, may I ask how you pass memory-backend-memfd ? Actually we have a cartesian param "vm_mem_backend" to be used for assign the memory backend, you can set it as vm_mem_backend = memory-backend-memfd and then have a try, then memory-backend-ram will not be used

Thanks for getting back.

I was using backend_mem=memory-backend-memfd, now I ran with vm_mem_backend = memory-backend-memfd -> it did not make any difference I continue to see memory-backend-ram during vm run.

I am listing configs I use from a memory and vm type below, please let me know your config:

use_mem = no
vm_secure_guest_type = snp
vm_sev_cbitpos = 51
vm_sev_reduced_phys_bits = 1
vm_mem_backend = memory-backend-memfd
mem_devs = "upm0"
vm_sev_upm_devs = "upm0"
vm_sev_upm_memdev_upm0 = upm0
size_mem_upm0 = 4096M

I see, vm_mem_backend is used for system memory, i.e. you will see the object id in machine option (e.g. -machine q35, xxx, memory-backend=mem_backend_obj_id). You defined the mem_devs, but set use_mem = no, which means no memory device will be generated.

I cannot figure out what happened to your code, would you paste your code change, or the qemu command line in your debug log?

@bssrikanth
Copy link
Contributor

@zhencliu I am testing out this patch on top of avocado-vt master, there is no custom code which I have added.

Common config:

vm_secure_guest_type = snp
mem_devs = "upm0"
backend_mem = "memory-backend-memfd"
vm_mem_backend = memory-backend-memfd
share_upm0 = true
prealloc_upm0 = false
reserve_upm0 = false
vm_sev_upm_devs = "upm0"
vm_sev_upm_memdev_upm0 = upm0
size_mem_upm0 = 4096M

Using above common config without use_mem = no hit issue mentioned below:

[stdlog] MALLOC_PERTURB_=1  /home/VT_BUILD/usr/local/bin/qemu-system-x86_64 \
[stdlog]     -machine q35,confidential-guest-support=lsec0,memory-backend=mem-machine_mem \
[stdlog]     -object sev-snp-guest,id=lsec0,cbitpos=51,reduced-phys-bits=1,kernel-hashes=off  \
[stdlog]     -m 1024 \
[stdlog]     -object memory-backend-memfd,size=4096M,prealloc=false,share=true,reserve=false,id=mem-upm0 \
[stdlog]     -device pc-dimm,id=dimm-upm0,memdev=mem-upm0 \
[stdlog]     -object memory-backend-memfd,size=1024M,id=mem-machine_mem  \
..
[stdlog] 2024-10-24 07:15:05,695 avocado.virttest.qemu_vm client           L0635 INFO | [qemu output] qemu-system-x86_64: -device pc-dimm,id=dimm-upm0,memdev=mem-upm0: no slots where allocated, please specify the 'slots' option

Using common config with use_mem = no I see the SNP boot test passing, below are the mem related commandline generated:

[stdlog] MALLOC_PERTURB_=1  /home/VT_BUILD/usr/local/bin/qemu-system-x86_64 \
[stdlog]     -machine q35,confidential-guest-support=lsec0,memory-backend=mem-machine_mem \
[stdlog]     -object sev-snp-guest,id=lsec0,cbitpos=51,reduced-phys-bits=1,kernel-hashes=off  \
[stdlog]     -m 1024 \
[stdlog]     -object memory-backend-memfd,size=4096M,prealloc=false,share=true,reserve=false,id=mem-upm0 \
[stdlog]     -object memory-backend-memfd,size=1024M,id=mem-machine_mem  \
..

@zhencliu
Copy link
Contributor Author

@zhencliu I am testing out this patch on top of avocado-vt master, there is no custom code which I have added.

Common config:

vm_secure_guest_type = snp
mem_devs = "upm0"
backend_mem = "memory-backend-memfd"
vm_mem_backend = memory-backend-memfd
share_upm0 = true
prealloc_upm0 = false
reserve_upm0 = false
vm_sev_upm_devs = "upm0"
vm_sev_upm_memdev_upm0 = upm0
size_mem_upm0 = 4096M

Using above common config without use_mem = no hit issue mentioned below:

[stdlog] MALLOC_PERTURB_=1  /home/VT_BUILD/usr/local/bin/qemu-system-x86_64 \
[stdlog]     -machine q35,confidential-guest-support=lsec0,memory-backend=mem-machine_mem \
[stdlog]     -object sev-snp-guest,id=lsec0,cbitpos=51,reduced-phys-bits=1,kernel-hashes=off  \
[stdlog]     -m 1024 \
[stdlog]     -object memory-backend-memfd,size=4096M,prealloc=false,share=true,reserve=false,id=mem-upm0 \
[stdlog]     -device pc-dimm,id=dimm-upm0,memdev=mem-upm0 \
[stdlog]     -object memory-backend-memfd,size=1024M,id=mem-machine_mem  \
..
[stdlog] 2024-10-24 07:15:05,695 avocado.virttest.qemu_vm client           L0635 INFO | [qemu output] qemu-system-x86_64: -device pc-dimm,id=dimm-upm0,memdev=mem-upm0: no slots where allocated, please specify the 'slots' option

Use slots_mem param to set the slots


Using common config with use_mem = no I see the SNP boot test passing, below are the mem related commandline generated:

[stdlog] MALLOC_PERTURB_=1 /home/VT_BUILD/usr/local/bin/qemu-system-x86_64
[stdlog] -machine q35,confidential-guest-support=lsec0,memory-backend=mem-machine_mem
[stdlog] -object sev-snp-guest,id=lsec0,cbitpos=51,reduced-phys-bits=1,kernel-hashes=off
[stdlog] -m 1024
[stdlog] -object memory-backend-memfd,size=4096M,prealloc=false,share=true,reserve=false,id=mem-upm0
[stdlog] -object memory-backend-memfd,size=1024M,id=mem-machine_mem
..

So what is upm0 used for? Can we remove it? I mean the following configuration is OK for memory

vm_secure_guest_type = snp
vm_sev_cbitpos = 51
vm_sev_reduced_phys_bits = 1
vm_mem_backend = memory-backend-memfd

Would you give your expected qemu command line for memory device? Then we can show you some sample configurations, currently we are confused for your usage of memory

@bssrikanth
Copy link
Contributor

bssrikanth commented Oct 25, 2024

Would you give your expected qemu command line for memory device? Then we can show you some sample configurations, currently we are confused for your usage of memory

@zhencliu there were some confusions, now its clear. I have tested SNP boot with memfd successfully with this patch. I am now in process of trying out different memory configurations. I will let you know here how it goes. Thank you for your time till now!

@bssrikanth
Copy link
Contributor

@zhencliu I could able to test this patch out in my environment with different memory topologies.
Tested-by: Srikanth Aithal [email protected]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants