rootBusPath: create rootBusPath dynamically. #860

jongwu · 2020-10-26T06:53:30Z

Currently, rootBusPath is set as a constant value. Bus this pcie bus path
is not alway static, eg. the rootBus on arm64 is
"/devices/platform/4010000000.pcie/pci0000:00". The address part of
"4010000000.pcie" may vary with the maxMem in qemu so it should not be a
fixed value.

For exmaple:
HIGH PCIE address reserved in qemu on virt is from 0 to 512G. The lower
part of address may be allocated to normal memory. In general, normal memory
is largely less than 256G, so the base address of HIGH PCIE can be 256G.
But, in case the maxmem is large enough, like around 256G, the base address
of HIGH PCIE must be increase, eg. 300G.
In the current implementation of kata-runtime, the maxmem in qemu is the host
memory size. So if the host memory size is large enough, the prefix of the
pci device path name will be different from rootBusPath set in kata-agent which
can lead to failure on device hotplug.

This patch offer a mechanism to create rootBusPath dynamically but only give
implemention for arm64 and return the default value of rootBusPath for other arch.

Fixes: #859
Signed-off-by: Jianyong Wu jianyong.wu@arm.com

@jodh-intel @devimc

codecov · 2020-10-26T07:02:36Z

Codecov Report

Merging #860 (d66fcb8) into master (5cfb8ec) will increase coverage by 0.08%.
The diff coverage is 60.00%.

@@            Coverage Diff             @@
##           master     #860      +/-   ##
==========================================
+ Coverage   57.72%   57.81%   +0.08%     
==========================================
  Files          17       19       +2     
  Lines        2370     2375       +5     
==========================================
+ Hits         1368     1373       +5     
  Misses        840      840              
  Partials      162      162

alicefr · 2020-10-26T09:00:20Z

cc @jschintag

dgibson

I think this is a good idea. ppc64 doesn't need this just yet, but at some point it will need the ability to handle multiple PCI roots, and this gets us a little closer to doing that.

devimc · 2020-10-26T13:14:47Z

/test

devimc

thanks @jongwu

jodh-intel

Hi @jongwu - thanks for raising. I've added some comments so blocking PR for now...

jodh-intel · 2020-10-27T09:26:00Z

device_arm64.go

+		fmt.Errorf("Error reading %s: %s", sysStartRootBusPath, err)
+	}
+
+	// find out the director end with ".pcie"


Is this a typo ("directory" rather than "director")?

jodh-intel · 2020-10-27T09:26:37Z

device_arm64.go

+
+	// find out the director end with ".pcie"
+	for _, file := range files {
+		if strings.HasSuffix(file.Name(), ".pcie") {


If the comment above has a typo, shouldn't this also test that file.IsDir() == true?

jodh-intel · 2020-10-27T09:28:39Z

device_arm64.go

+		}
+	}
+
+	return ""


What does returning "" mean? Is that a valid scenario or should this function actually return (string, error)?

jodh-intel · 2020-10-27T09:29:09Z

device_arm64.go

+	sysStartRootBusPath := filepath.Join(sysfsDir, startRootBusPath)
+	files, err := ioutil.ReadDir(sysStartRootBusPath)
+	if err != nil {
+		fmt.Errorf("Error reading %s: %s", sysStartRootBusPath, err)


This suggests the function should be returning an error to me...?

jodh-intel

Thanks @jongwu.

lgtm

jodh-intel · 2020-10-27T11:32:16Z

/test

jodh-intel · 2020-10-27T11:33:01Z

@jongwu - Travis is failing - looks like you need to update the tests for the latest changes.

jongwu · 2020-10-27T12:18:45Z

thanks @jodh-intel , I have fixed it. now could you please retrigger ci again?

jodh-intel · 2020-10-27T13:29:23Z

/test

jongwu · 2020-10-27T13:56:05Z

hello @jodh-intel -, I can't see ARM CI, could please trigger it?

jodh-intel · 2020-10-27T14:08:50Z

/test-arm

@jongwu - I've added you to the runtime + agent repos so you should be able to trigger builds yourself from now on. Also, are you planning to port this to the new 2.0 rust agent?

amshinde · 2020-10-27T20:53:07Z

/test-arm

amshinde · 2020-10-27T20:54:32Z

Dont see an ARM CI.
@chavafg Is it not enabled for the agent?

chavafg · 2020-10-27T23:04:33Z

hmm, yeah, it seems that arm CI has not been enabled for this repo. @jongwu could you help adding it? let me know if you need help.

jongwu · 2020-10-28T02:15:18Z

/test-arm

jongwu · 2020-10-28T05:05:38Z

/test

jongwu · 2020-10-28T06:45:54Z

/test

Currently, rootBusPath is set as a constant value. Bus this pcie bus path is not alway static, eg. the rootBus on arm64 is "/devices/platform/4010000000.pcie/pci0000:00". The address part of "4010000000.pcie" may vary with the maxMem in qemu so it should not be a fixed value. For exmaple: HIGH PCIE address reserved in qemu on virt is from 0 to 512G. The lower part of address may be allocated to normal memory. In general, normal memory is largely less than 256G, so the base address of HIGH PCIE can be 256G. But, in case the maxmem is large enough, like around 256G, the base address of HIGH PCIE must be increase, eg. 300G. In the current implementation of kata-runtime, the maxmem in qemu is the host memory size. So if the host memory size is large enough, the prefix of the pci device path name will be different from rootBusPath set in kata-agent which can lead to failure on device hotplug. This patch offer a mechanism to create rootBusPath dynamically but only give implemention for arm64 and return the default value of rootBusPath for other arch. Fixes: kata-containers#859 Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>

jongwu · 2020-10-28T06:46:49Z

/test

port kata-containers/agent#860 here. Fixes: kata-containers#1059 Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>

jongwu · 2020-10-30T08:53:44Z

Thanks @jodh-intel -, I have composed the rust version of this patch, see rust. It's not easy for me to write rust code.

jodh-intel · 2020-10-30T09:03:47Z

Thanks @jongwu! No problem - we can help if necessary 😄

port kata-containers/agent#860 here. Fixes: kata-containers#1059 Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>

jongwu · 2020-10-30T09:37:05Z

thanks @jodh-intel .

port kata-containers/agent#860 here. Fixes: kata-containers#1059 Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>

jongwu · 2020-11-26T03:38:40Z

Hello @jodh-intel @chavafg, all tests pass. isn't time to merge this?

dgibson approved these changes Oct 26, 2020

View reviewed changes

devimc approved these changes Oct 26, 2020

View reviewed changes

jodh-intel suggested changes Oct 27, 2020

View reviewed changes

jongwu force-pushed the rootbus branch from 699db0a to 34221c6 Compare October 27, 2020 10:08

jodh-intel approved these changes Oct 27, 2020

View reviewed changes

jongwu force-pushed the rootbus branch from 34221c6 to 6f9ddc8 Compare October 27, 2020 11:34

jodh-intel added the needs-forward-port Changes need to be applied to a newer branch / repository label Oct 27, 2020

amshinde approved these changes Oct 27, 2020

View reviewed changes

jongwu force-pushed the rootbus branch from 6f9ddc8 to 1a15205 Compare October 28, 2020 05:04

jongwu force-pushed the rootbus branch from 44c563d to d66fcb8 Compare October 28, 2020 06:46

jongwu added a commit to jongwu/kata-containers that referenced this pull request Oct 30, 2020

agent: create pci root Bus Path for arm64

8376088

port kata-containers/agent#860 here. Fixes: kata-containers#1059 Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>

jongwu mentioned this pull request Oct 30, 2020

agent: create pci root Bus Path for arm64 kata-containers/kata-containers#1060

Merged

jongwu added a commit to jongwu/kata-containers that referenced this pull request Oct 30, 2020

agent: create pci root Bus Path for arm64

3ca0a4b

port kata-containers/agent#860 here. Fixes: kata-containers#1059 Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>

jongwu added a commit to jongwu/kata-containers that referenced this pull request Nov 3, 2020

agent: create pci root Bus Path for arm64

68e9d87

port kata-containers/agent#860 here. Fixes: kata-containers#1059 Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>

jongwu added a commit to jongwu/kata-containers that referenced this pull request Nov 3, 2020

agent: create pci root Bus Path for arm64

a41812d

port kata-containers/agent#860 here. Fixes: kata-containers#1059 Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>

jongwu added a commit to jongwu/kata-containers that referenced this pull request Nov 3, 2020

agent: create pci root Bus Path for arm64

393baf0

port kata-containers/agent#860 here. Fixes: kata-containers#1059 Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>

jongwu added a commit to jongwu/kata-containers that referenced this pull request Nov 4, 2020

agent: create pci root Bus Path for arm64

e7504cb

port kata-containers/agent#860 here. Fixes: kata-containers#1059 Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>

jongwu added a commit to jongwu/kata-containers that referenced this pull request Nov 4, 2020

agent: create pci root Bus Path for arm64

1625bec

port kata-containers/agent#860 here. Fixes: kata-containers#1059 Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>

jongwu added a commit to jongwu/kata-containers that referenced this pull request Nov 5, 2020

agent: create pci root Bus Path for arm64

6c2fc23

port kata-containers/agent#860 here. Fixes: kata-containers#1059 Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>

devimc merged commit 25d7471 into kata-containers:master Nov 26, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rootBusPath: create rootBusPath dynamically. #860

rootBusPath: create rootBusPath dynamically. #860

jongwu commented Oct 26, 2020

codecov bot commented Oct 26, 2020 •

edited

Loading

alicefr commented Oct 26, 2020

dgibson left a comment

devimc commented Oct 26, 2020

devimc left a comment

jodh-intel left a comment

jodh-intel Oct 27, 2020

jodh-intel Oct 27, 2020

jodh-intel Oct 27, 2020

jodh-intel Oct 27, 2020

jodh-intel left a comment

jodh-intel commented Oct 27, 2020

jodh-intel commented Oct 27, 2020

jongwu commented Oct 27, 2020

jodh-intel commented Oct 27, 2020

jongwu commented Oct 27, 2020

jodh-intel commented Oct 27, 2020

amshinde commented Oct 27, 2020

amshinde commented Oct 27, 2020

chavafg commented Oct 27, 2020

jongwu commented Oct 28, 2020

jongwu commented Oct 28, 2020

jongwu commented Oct 28, 2020

jongwu commented Oct 28, 2020

jongwu commented Oct 30, 2020

jodh-intel commented Oct 30, 2020

jongwu commented Oct 30, 2020

jongwu commented Nov 26, 2020

rootBusPath: create rootBusPath dynamically. #860

rootBusPath: create rootBusPath dynamically. #860

Conversation

jongwu commented Oct 26, 2020

codecov bot commented Oct 26, 2020 • edited Loading

Codecov Report

alicefr commented Oct 26, 2020

dgibson left a comment

Choose a reason for hiding this comment

devimc commented Oct 26, 2020

devimc left a comment

Choose a reason for hiding this comment

jodh-intel left a comment

Choose a reason for hiding this comment

jodh-intel Oct 27, 2020

Choose a reason for hiding this comment

jodh-intel Oct 27, 2020

Choose a reason for hiding this comment

jodh-intel Oct 27, 2020

Choose a reason for hiding this comment

jodh-intel Oct 27, 2020

Choose a reason for hiding this comment

jodh-intel left a comment

Choose a reason for hiding this comment

jodh-intel commented Oct 27, 2020

jodh-intel commented Oct 27, 2020

jongwu commented Oct 27, 2020

jodh-intel commented Oct 27, 2020

jongwu commented Oct 27, 2020

jodh-intel commented Oct 27, 2020

amshinde commented Oct 27, 2020

amshinde commented Oct 27, 2020

chavafg commented Oct 27, 2020

jongwu commented Oct 28, 2020

jongwu commented Oct 28, 2020

jongwu commented Oct 28, 2020

jongwu commented Oct 28, 2020

jongwu commented Oct 30, 2020

jodh-intel commented Oct 30, 2020

jongwu commented Oct 30, 2020

jongwu commented Nov 26, 2020

codecov bot commented Oct 26, 2020 •

edited

Loading